Skip to content

Allow audit URLs to be used instead of diffs - #13

Open
adam-ja wants to merge 4 commits into
rhysr:masterfrom
adam-ja:master
Open

Allow audit URLs to be used instead of diffs#13
adam-ja wants to merge 4 commits into
rhysr:masterfrom
adam-ja:master

Conversation

@adam-ja

@adam-ja adam-ja commented Nov 14, 2017

Copy link
Copy Markdown

Sometimes, we need to release quick fixes that were not reviewed. We still want to add these to the release app for visibility and, rather than linking to nothing, we can link to the audit on phab.

For example, link to http://phab.dev.ebuyer.com/rGENONEfcc395703a1f8edee04db518ed56c121705900fe rather than http://phab.dev.ebuyer.com/D5610

This patch updates the Add Diff form to indicate this possibility, and improves the styling of the release view so that lengthy commit hashes do not overflow other elements of a release (such as the Release button/Released status text).

Adam Auskerin added 3 commits February 2, 2017 15:38
Previously the cut off warning would always appear if the current hour
was 14 or higher. This meant that a release two or more days into the
future could show the warning prematurely (e.g. we should be able to add
tickets to a release on Wednesday after 2pm on Monday but the warning
was still showing).

Now, the cut off warning actually takes into account the date of the
release and only shows the warning after 2pm on the last working day
before the release.
Sometimes, we need to release quick fixes that were not reviewed. We
still want to add these to the release app for visibility and, rather
than linking to nothing, we can link to the audit on phab.

For example, link to
http://phab.dev.ebuyer.com/rGENONEfcc395703a1f8edee04db518ed56c121705900fe
rather than http://phab.dev.ebuyer.com/D5610

This patch updates the Add Diff form to indicate this possibility, and
improves the styling of the release view so that lengthy commit hashes
do not overflow other elements of a release (such as the Release
button/Released status text).
@adam-ja

adam-ja commented Nov 14, 2017

Copy link
Copy Markdown
Author

BEFORE:
image
image

AFTER:
image
image

)
div(class='form-group')
label(class='control-label col-sm-2') Phabricator Diff
label(class='control-label col-sm-2') Phabricator Diff/Audit

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessarily a link to an "Audit". It's a link to a "Commit", sometimes those commits require Audit.
(shrug)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is indeed the case and, depending on which repo is being committed to, the audit may not be created immediately. "Commit" would be more appropriate terminology.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

We can link to any commit on phabricator, regardless of whether or not
that commit requires auditing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants