This field matches accepted or closed revisions, but essentially all revisions will be closed by the time we evaluate the rule.
Users who are interested in this rule usually mean "before this revision was autoclosed, was it accepted?". We probably have to store a separate flag for this (or introduce separate closed-from-accepted and closed-by-force states).
Use case for me is creating a herald rule to match revisions which were merged without ever being reviewed so they can instead be audits. Sometimes no one is available to do reviews and I need to more forward so I give up waiting and merge without review, but I still want someone to look at the commit eventually.
From IRC, a user reported a similar issue with pre-commit rules, which is surprising offhand:
I made a Herald rule: when all conditions are matching: 1) has a differential revision 2) does not have an accepted differential revision -> block change. It didn't block my commit :(
Let's make this check the state the revision had before we acted on it:
It'd be nice if this could detect based on the commit content (hashing it), rather than explicitly only allowing one commit per accepted revision.
In our scenario, a bug that applies to both nightly and long term service has to be diff'd twice in order to pass the "only land accepted commits rule". This is because when a developer diff's the bug fix and lands it on master, that diff will no longer count when cherry-picking to LTS.
It does, but the revision is already in a closed status, so it doesn't count when performing the rule (or at least it didn't when we started using it). The exact sequence is something like this:
The rule should identify closed revisions:
case self::FIELD_DIFFERENTIAL_ACCEPTED:
$revision = $this->loadDifferentialRevision();
if (!$revision) {
return null;
}
switch ($revision->getStatus()) {
case ArcanistDifferentialRevisionStatus::ACCEPTED:
case ArcanistDifferentialRevisionStatus::CLOSED:
return $revision->getPHID();
}
return null;This specific behavior changed in D8476 (circa March 10) so maybe your tests were from before that?
If we return rule as follows
When [all of] these conditions are met:
[Accepted Differential revision][does not exist]
Take these actions every time this rule matches:
[Block change with message][Review is required for all changes.]
You may want to use additional conditions like this, to run the rule only in certain repositories:
[Repository][is any of][ ... list of review-requied repositories ... ]
Or a condition like this, to let users bypass the rule by writing some string like "@bypass-review" in a message in an emergency:
[Body][does not contain][@bypass-review]
if we are using sourcetree as git client, what is the sample message for commit.