Page MenuHomePhabricator

When the push phase of "Land Revision" fails, show the error in the UI
ClosedPublic

Authored by nickz on Feb 26 2016, 5:36 AM.
Tags
None
Referenced Files
F13146367: D15350.diff
Fri, May 3, 10:01 AM
Unknown Object (File)
Mon, Apr 29, 4:54 PM
Unknown Object (File)
Mon, Apr 29, 4:54 PM
Unknown Object (File)
Mon, Apr 29, 4:54 PM
Unknown Object (File)
Mon, Apr 29, 4:54 PM
Unknown Object (File)
Thu, Apr 25, 5:41 PM
Unknown Object (File)
Thu, Apr 25, 3:25 AM
Unknown Object (File)
Fri, Apr 19, 4:15 PM
Tokens
"Like" token, awarded by epriestley.

Details

Summary
Test Plan

tested on my dev instance

Diff Detail

Repository
rP Phabricator
Branch
Show_Push_in_UI
Lint
Lint Passed
Unit
Tests Skipped
Build Status
Buildable 10851
Build 13377: arc lint + arc unit

Event Timeline

nickz retitled this revision from to When the push phase of "Land Revision" fails, show the error in the UI.
nickz updated this object.
nickz edited the test plan for this revision. (Show Details)
nickz added a reviewer: epriestley.
nickz edited edge metadata.
  • remove the change made by accident
epriestley edited edge metadata.

This looks great, thanks! One minor typo inline.

src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
89
  • Small typo: "custome" -> "custom"
  • Maybe don't say "git" because we'll probably use the same message for Mercurial in the future.
  • This probably reads a little more naturally as "...commit hook has failed" than "...commit hooks fail".

So maybe something like this, overall:

The push failed. This usually indicates that the change is breaking some rules or some custom commit hook has failed.

This revision is now accepted and ready to land.Feb 26 2016, 2:20 PM
This revision was automatically updated to reflect the committed changes.
nickz marked an inline comment as done.
joshuaspence added inline comments.
src/applications/drydock/DrydockCommandError/DrydockCommandError.php
4

Hm, I'm surprised that the linter did not complain about this... This class should be marked ad final and should extend from Phobject.

src/applications/drydock/operation/DrydockLandRepositoryOperation.php
132

It would be better to move this to a variable to avoid code duplication.

133

Wrong indentation.