When a commit is pushed to Gerrit with a bug number reference, Gerrit Notification Bot will first add a comment on that bug linking to the gerrit change. Then it will come back and set the status to PATCH_TO_REVIEW in a separate operation.
This leads to a superfluous second bugmail message whenever a commit is pushed.
Instead, the bot should be doing these two things in the one go.
Andre tells me that Christian thinks this could be hard to fix.
Version: wmf-deployment
Severity: normal
(In reply to comment #0)
[ Adding link to gerrit and setting PATCH__TO_REVIEW are separate
actions ]
This splitting is an inherent limitation of the original design of
gerrit's hooks-its plugin. With some effort, we could work around that
from within hooks-bugzilla, but hooks-its' approach also comes with a
benefit: If one of the two actions fail, the other is still carried
out.
If they were tied to a single request, they'd either both fail or both
work.
Currently, there has been some discussion about from which states one
could/would/should allow switch to PATCH_TO_REVIEW.
Forbidding this for some states means that setting the status might
fail, but we want the comment to be added nonetheless. So until the
workflow is fully settled, I guess we should not try to merge the two
separate requests into a single one.
Well ... maybe that has settled already? Andre?
This leads to a superfluous second bugmail message whenever a commit is
pushed.
Yes, that's true.
One can tune bugzilla's noisiness in the User preferences:
https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
allows to opt out of getting emails for "The priority, status,
severity, or milestone changes".
That mostly mitigates the problem, but does not solve it.
(In reply to comment #2)
Currently, there has been some discussion about from which states one
could/would/should allow switch to PATCH_TO_REVIEW.Well ... maybe that has settled already? Andre?
Currently we allow setting PATCH_TO_REVIEW from UNCONFIRMED, NEW, ASSIGNED, REOPENED, RESOLVED. For the time being I don't plan any changes, I'll see in the future if there could be any potential reasons to adjust this again.
(In reply to comment #3)
Currently, there has been some discussion about from which states one
could/would/should allow switch to PATCH_TO_REVIEW.
Well ... maybe that has settled already? Andre?
Currently we allow setting PATCH_TO_REVIEW from UNCONFIRMED, NEW, ASSIGNED,
REOPENED, RESOLVED. For the time being I don't plan any changes, I'll see in
the future if there could be any potential reasons to adjust this again.
Is this the only point where combining the two operations could fail?
(In reply to comment #5)
I don't understand the question - what is meant by "point"?
Christian said:
(In reply to comment #2)
(In reply to comment #0)
[ Adding link to gerrit and setting PATCH__TO_REVIEW are separate
actions ]
This splitting is an inherent limitation of the original design of
gerrit's hooks-its plugin. With some effort, we could work around that
from within hooks-bugzilla, but hooks-its' approach also comes with a
benefit: If one of the two actions fail, the other is still carried
out.
If they were tied to a single request, they'd either both fail or both
work.
[...]
Are there other scenarios that need to be addressed besides the allowed state => PATCH_TO_REVIEW issue, or does this mean that once the policy question is settled ("these are the allowed state transitions and we guarantee them UFN") this bug can be fixed? For example, do we need upstream (Gerrit) changes as a prerequisite?
I don't plan changes to the setting for "transitions are allowed from statuses X, Y and Z to status PATCH_TO_REVIEW", and this configuration setting in Bugzilla is independent from the code problem to solve in the bugzilla-hooks codebase that this bug report is about.
I think the minor disadvantage of potentially missing a change in Bugzilla due to some sort of error on rare occasions, is substantially outweighed by the annoyance of an extra email *every* time a patch is submitted...
Now that we've migrated from Bugzilla to Phabricator, the bot leaving those comments and adding the Patch-For-Review tag has been updated and from what I can see it trigger separate notifications now.