From 5042667b96fc98977cee471976235cacffb20de1 Mon Sep 17 00:00:00 2001 From: Nick Harper Date: Mon, 27 Feb 2012 16:09:22 -0800 Subject: [PATCH] Improved warning message when accepting diff with skipped lint or unit Summary: Some people find the current message stating "This diff has Lint/Unit Test Problems" confusing if the unit tests or lint was skipped. This revision clarifies those messages. Test Plan: Started to accept a revision with skipped lint and unit tests, and saw the new message. Reviewers: epriestley, btrahan, jungejason Reviewed By: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1738 --- .../addcomment/DifferentialAddCommentView.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php index f19e929652..1527e81ee2 100644 --- a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php +++ b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php @@ -149,9 +149,15 @@ final class DifferentialAddCommentView extends AphrontView { DifferentialLintStatus::LINT_FAIL => 'Lint Failure', DifferentialLintStatus::LINT_SKIP => 'Lint Skipped' ); - $content = - "

This diff has Lint Problems. Make sure you are OK with them ". - "before you accept this diff.

"; + if ($diff->getLintStatus() == DifferentialLintStatus::LINT_SKIP) { + $content = + "

This diff was created without running lint. Make sure you are ". + "OK with that before you accept this diff.

"; + } else { + $content = + "

This diff has Lint Problems. Make sure you are OK with them ". + "before you accept this diff.

"; + } $lint_warning = $this->generateWarningView( $diff->getLintStatus(), $titles, @@ -172,6 +178,10 @@ final class DifferentialAddCommentView extends AphrontView { "

This diff has postponed unit tests. The results should be ". "coming in soon. You should probably wait for them before accepting ". "this diff.

"; + } else if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_SKIP) { + $content = + "

Unit tests were skipped when this diff was created. Make sure ". + "you are OK with that before you accept this diff.

"; } else { $content = "

This diff has Unit Test Problems. Make sure you are OK with ".