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
Unknown Object (File)
Wed, Dec 18, 1:12 AM
Unknown Object (File)
Wed, Dec 18, 1:12 AM
Unknown Object (File)
Wed, Dec 18, 1:12 AM
Unknown Object (File)
Tue, Dec 17, 6:53 AM
Unknown Object (File)
Thu, Dec 12, 7:49 AM
Unknown Object (File)
Sun, Dec 8, 2:42 PM
Unknown Object (File)
Fri, Dec 6, 10:05 AM
Unknown Object (File)
Mon, Dec 2, 10:48 PM
Tokens
"Like" token, awarded by epriestley.

Details

Summary
Test Plan

tested on my dev instance

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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
3

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.