Add an "Accepting reviewers" Herald field for commits
Summary:
See PHI262. Fixes T12578. Although this is a bit niche and probably better accomplished through advisory/soft measures ("Add blocking reviewers") in most cases, it isn't difficult to implement and doesn't create any technical or product tension.
If installs write a rule that blocks commits, that will probably also naturally lead them to an "add reviewers" rule anyway.
Also, allow packages to be hit with the typeahead. They're valid reviewers but previously you couldn't write rules against them, for no actual reason.
Test Plan: Used test console to run this against commits, got sensible results for the field value.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12578
Differential Revision: https://secure.phabricator.com/D18839
This commit is contained in:
@@ -696,6 +696,7 @@ phutil_register_library_map(array(
|
||||
'DiffusionCommitRevertsCommitEdgeType' => 'applications/diffusion/edge/DiffusionCommitRevertsCommitEdgeType.php',
|
||||
'DiffusionCommitReviewerHeraldField' => 'applications/diffusion/herald/DiffusionCommitReviewerHeraldField.php',
|
||||
'DiffusionCommitRevisionAcceptedHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptedHeraldField.php',
|
||||
'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionAcceptingReviewersHeraldField.php',
|
||||
'DiffusionCommitRevisionHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionHeraldField.php',
|
||||
'DiffusionCommitRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionReviewersHeraldField.php',
|
||||
'DiffusionCommitRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionCommitRevisionSubscribersHeraldField.php',
|
||||
@@ -809,6 +810,7 @@ phutil_register_library_map(array(
|
||||
'DiffusionPreCommitContentRepositoryHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRepositoryHeraldField.php',
|
||||
'DiffusionPreCommitContentRepositoryProjectsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRepositoryProjectsHeraldField.php',
|
||||
'DiffusionPreCommitContentRevisionAcceptedHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptedHeraldField.php',
|
||||
'DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField.php',
|
||||
'DiffusionPreCommitContentRevisionHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionHeraldField.php',
|
||||
'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php',
|
||||
'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionSubscribersHeraldField.php',
|
||||
@@ -5755,6 +5757,7 @@ phutil_register_library_map(array(
|
||||
'DiffusionCommitRevertsCommitEdgeType' => 'PhabricatorEdgeType',
|
||||
'DiffusionCommitReviewerHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRevisionAcceptedHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRevisionAcceptingReviewersHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRevisionHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRevisionReviewersHeraldField' => 'DiffusionCommitHeraldField',
|
||||
'DiffusionCommitRevisionSubscribersHeraldField' => 'DiffusionCommitHeraldField',
|
||||
@@ -5871,6 +5874,7 @@ phutil_register_library_map(array(
|
||||
'DiffusionPreCommitContentRepositoryHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRepositoryProjectsHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRevisionAcceptedHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRevisionHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'DiffusionPreCommitContentHeraldField',
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
<?php
|
||||
|
||||
final class DiffusionCommitRevisionAcceptingReviewersHeraldField
|
||||
extends DiffusionCommitHeraldField {
|
||||
|
||||
const FIELDCONST = 'diffusion.commit.revision.accepting';
|
||||
|
||||
public function getHeraldFieldName() {
|
||||
return pht('Accepting reviewers');
|
||||
}
|
||||
|
||||
public function getFieldGroupKey() {
|
||||
return HeraldRelatedFieldGroup::FIELDGROUPKEY;
|
||||
}
|
||||
|
||||
public function getHeraldFieldValue($object) {
|
||||
$revision = $this->getAdapter()->loadDifferentialRevision();
|
||||
|
||||
if (!$revision) {
|
||||
return array();
|
||||
}
|
||||
|
||||
$diff_phid = $revision->getActiveDiffPHID();
|
||||
|
||||
$reviewer_phids = array();
|
||||
foreach ($revision->getReviewers() as $reviewer) {
|
||||
if ($reviewer->isAccepted($diff_phid)) {
|
||||
$reviewer_phids[] = $reviewer->getReviewerPHID();
|
||||
}
|
||||
}
|
||||
|
||||
return $reviewer_phids;
|
||||
}
|
||||
|
||||
protected function getHeraldFieldStandardType() {
|
||||
return self::STANDARD_PHID_LIST;
|
||||
}
|
||||
|
||||
protected function getDatasource() {
|
||||
return new DifferentialReviewerDatasource();
|
||||
}
|
||||
|
||||
}
|
||||
@@ -28,7 +28,7 @@ final class DiffusionCommitRevisionReviewersHeraldField
|
||||
}
|
||||
|
||||
protected function getDatasource() {
|
||||
return new PhabricatorProjectOrUserDatasource();
|
||||
return new DifferentialReviewerDatasource();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
<?php
|
||||
|
||||
final class DiffusionPreCommitContentRevisionAcceptingReviewersHeraldField
|
||||
extends DiffusionPreCommitContentHeraldField {
|
||||
|
||||
const FIELDCONST = 'diffusion.pre.content.revision.accepting';
|
||||
|
||||
public function getHeraldFieldName() {
|
||||
return pht('Accepting reviewers');
|
||||
}
|
||||
|
||||
public function getFieldGroupKey() {
|
||||
return HeraldRelatedFieldGroup::FIELDGROUPKEY;
|
||||
}
|
||||
|
||||
public function getHeraldFieldValue($object) {
|
||||
$revision = $this->getAdapter()->getRevision();
|
||||
|
||||
if (!$revision) {
|
||||
return array();
|
||||
}
|
||||
|
||||
$diff_phid = $revision->getActiveDiffPHID();
|
||||
|
||||
$reviewer_phids = array();
|
||||
foreach ($revision->getReviewers() as $reviewer) {
|
||||
if ($reviewer->isAccepted($diff_phid)) {
|
||||
$reviewer_phids[] = $reviewer->getReviewerPHID();
|
||||
}
|
||||
}
|
||||
|
||||
return $reviewer_phids;
|
||||
}
|
||||
|
||||
protected function getHeraldFieldStandardType() {
|
||||
return self::STANDARD_PHID_LIST;
|
||||
}
|
||||
|
||||
protected function getDatasource() {
|
||||
return new DifferentialReviewerDatasource();
|
||||
}
|
||||
|
||||
}
|
||||
@@ -28,7 +28,7 @@ final class DiffusionPreCommitContentRevisionReviewersHeraldField
|
||||
}
|
||||
|
||||
protected function getDatasource() {
|
||||
return new PhabricatorProjectOrUserDatasource();
|
||||
return new DifferentialReviewerDatasource();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user