revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00
Code Issues Releases Wiki Activity

Added a big warning if reviewer is about to accept a diff with lint or unit

Browse source 
errors.

Summary:
Make sure reviewers know what they are doing.

Test Plan:
Tested with different diffs that had lint and unit problems.

Reviewed By: epriestley
Reviewers: epriestley, jungejason
CC: grglr, aran, epriestley, tuomaspelkonen
Differential Revision: 432
This commit is contained in:
tuomaspelkonen 2011-06-09 18:56:47 -07:00
parent eab768f705
commit 501c001520
5 changed files with 151 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
src/__celerity_resource_map__.php
View file

@ -16,15 +16,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/aphront/attached-file-view.css',
),
'aphront-crumbs-view-css' =>
array(
'uri' => '/res/9009e6bd/rsrc/css/aphront/crumbs-view.css',
'type' => 'css',
'requires' =>
array(
),
'disk' => '/rsrc/css/aphront/crumbs-view.css',
),
'aphront-dark-console-css' =>
array(
'uri' => '/res/e7011594/rsrc/css/aphront/dark-console.css',
@ -34,15 +25,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/aphront/dark-console.css',
),
'aphront-dialog-view-css' =>
array(
'uri' => '/res/61a58113/rsrc/css/aphront/dialog-view.css',
'type' => 'css',
'requires' =>
array(
),
'disk' => '/rsrc/css/aphront/dialog-view.css',
),
'aphront-error-view-css' =>
array(
'uri' => '/res/e4c5e4ed/rsrc/css/aphront/error-view.css',
@ -52,6 +34,34 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/css/aphront/error-view.css',
),
0 =>
array(
'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-install',
),
'disk' => '/rsrc/js/javelin/docs/Base.js',
),
'aphront-crumbs-view-css' =>
array(
'uri' => '/res/9009e6bd/rsrc/css/aphront/crumbs-view.css',
'type' => 'css',
'requires' =>
array(
),
'disk' => '/rsrc/css/aphront/crumbs-view.css',
),
'aphront-dialog-view-css' =>
array(
'uri' => '/res/61a58113/rsrc/css/aphront/dialog-view.css',
'type' => 'css',
'requires' =>
array(
),
'disk' => '/rsrc/css/aphront/dialog-view.css',
),
'aphront-form-view-css' =>
array(
'uri' => '/res/8ee16aba/rsrc/css/aphront/form-view.css',
@ -360,6 +370,17 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/core/behavior-dark-console.js',
),
'javelin-behavior-differential-accept-with-errors' =>
array(
'uri' => '/res/41c4685b/rsrc/js/application/differential/behavior-accept-with-errors.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-behavior',
1 => 'javelin-dom',
),
'disk' => '/rsrc/js/application/differential/behavior-accept-with-errors.js',
),
'javelin-behavior-differential-add-reviewers' =>
array(
'uri' => '/res/dc79790c/rsrc/js/application/differential/behavior-add-reviewers.js',
@ -590,16 +611,6 @@ celerity_register_resource_map(array(
),
'disk' => '/rsrc/js/application/owners/owners-path-editor.js',
),
0 =>
array(
'uri' => '/res/39de799e/rsrc/js/javelin/docs/Base.js',
'type' => 'js',
'requires' =>
array(
0 => 'javelin-install',
),
'disk' => '/rsrc/js/javelin/docs/Base.js',
),
'javelin-behavior-phabricator-keyboard-shortcuts' =>
array(
'uri' => '/res/5a23bcc8/rsrc/js/application/core/behavior-keyboard-shortcuts.js',

68
src/applications/differential/view/addcomment/DifferentialAddCommentView.php
View file

@ -47,6 +47,22 @@ final class DifferentialAddCommentView extends AphrontView {
return $this;
}
private function generateWarningView(
$status,
array $titles,
$id,
$content) {
$warning = new AphrontErrorView();
$warning->setSeverity(AphrontErrorView::SEVERITY_ERROR);
$warning->setWidth(AphrontErrorView::WIDTH_WIDE);
$warning->setID($id);
$warning->appendChild($content);
$warning->setTitle(idx($titles, $status, 'Warning'));
return $warning;
}
public function render() {
require_celerity_resource('differential-revision-add-comment-css');
@ -96,6 +112,52 @@ final class DifferentialAddCommentView extends AphrontView {
'row' => 'add-reviewers',
));
$diff = $revision->loadActiveDiff();
$lint_warning = null;
$unit_warning = null;
if ($diff->getLintStatus() >= DifferentialLintStatus::LINT_WARN) {
$titles =
array(
DifferentialLintStatus::LINT_WARN => 'Lint Warning',
DifferentialLintStatus::LINT_FAIL => 'Lint Failure',
DifferentialLintStatus::LINT_SKIP => 'Lint Skipped'
);
$content =
"<p>This diff has Lint Problems. Make sure you are OK with them ".
"before you accept this diff.</p>";
$lint_warning = $this->generateWarningView(
$diff->getLintStatus(),
$titles,
'lint-warning',
$content);
}
if ($diff->getUnitStatus() >= DifferentialUnitStatus::UNIT_WARN) {
$titles =
array(
DifferentialUnitStatus::UNIT_WARN => 'Unit Tests Warning',
DifferentialUnitStatus::UNIT_FAIL => 'Unit Tests Failure',
DifferentialUnitStatus::UNIT_SKIP => 'Unit Tests Skipped',
DifferentialUnitStatus::UNIT_POSTPONED => 'Unit Tests Postponed'
);
$content =
"<p>This diff has Unit Test Problems. Make sure you are OK with them ".
"before you accept this diff.</p>";
$unit_warning = $this->generateWarningView(
$diff->getUnitStatus(),
$titles,
'unit-warning',
$content);
}
Javelin::initBehavior(
'differential-accept-with-errors',
array(
'select' => 'comment-action',
'lint_warning' => $lint_warning ? 'lint-warning' : null,
'unit_warning' => $unit_warning ? 'unit-warning' : null,
));
$rev_id = $revision->getID();
Javelin::initBehavior(
@ -112,6 +174,12 @@ final class DifferentialAddCommentView extends AphrontView {
$panel_view = new AphrontPanelView();
$panel_view->appendChild($form);
if ($lint_warning) {
$panel_view->appendChild($lint_warning);
}
if ($unit_warning) {
$panel_view->appendChild($unit_warning);
}
$panel_view->setHeader('Leap Into Action');
$panel_view->addClass('aphront-panel-accent');
$panel_view->addClass('aphront-panel-flush');

3
src/applications/differential/view/addcomment/__init__.php
View file

@ -7,6 +7,8 @@
phutil_require_module('phabricator', 'applications/differential/constants/action');
phutil_require_module('phabricator', 'applications/differential/constants/lintstatus');
phutil_require_module('phabricator', 'applications/differential/constants/unitstatus');
phutil_require_module('phabricator', 'infrastructure/celerity/api');
phutil_require_module('phabricator', 'infrastructure/javelin/api');
phutil_require_module('phabricator', 'view/base');
@ -15,6 +17,7 @@ phutil_require_module('phabricator', 'view/form/control/select');
phutil_require_module('phabricator', 'view/form/control/submit');
phutil_require_module('phabricator', 'view/form/control/textarea');
phutil_require_module('phabricator', 'view/form/control/tokenizer');
phutil_require_module('phabricator', 'view/form/error');
phutil_require_module('phabricator', 'view/layout/panel');
phutil_require_module('phutil', 'utils');

17
src/view/form/error/AphrontErrorView.php
View file

@ -30,6 +30,7 @@ final class AphrontErrorView extends AphrontView {
private $errors;
private $severity;
private $width;
private $id;
public function setTitle($title) {
$this->title = $title;
@ -51,6 +52,11 @@ final class AphrontErrorView extends AphrontView {
return $this;
}
public function setID($id) {
$this->id = $id;
return $this;
}
final public function render() {
require_celerity_resource('aphront-error-view-css');
@ -85,11 +91,14 @@ final class AphrontErrorView extends AphrontView {
$more_classes = implode(' ', $more_classes);
return
'<div class="aphront-error-view '.$more_classes.'">'.
phutil_render_tag(
'div',
array(
'id' => $this->id,
'class' => 'aphront-error-view '.$more_classes,
),
$title.
$this->renderChildren().
$list.
'</div>';
$list);
}
}

28
webroot/rsrc/js/application/differential/behavior-accept-with-errors.js Normal file
View file

@ -0,0 +1,28 @@
/**
* @provides javelin-behavior-differential-accept-with-errors
* @requires javelin-behavior
* javelin-dom
*/
JX.behavior('differential-accept-with-errors', function(config) {
function toggleWarning(control) {
if (control) {
JX.DOM.hide(JX.$(control));
JX.DOM.listen(
JX.$(config.select),
'change',
null,
function(e) {
if (JX.$(config.select).value == 'accept') {
JX.DOM.show(JX.$(control));
} else {
JX.DOM.hide(JX.$(control));
}
});
}
}
toggleWarning(config.unit_warning);
toggleWarning(config.lint_warning);
});