Page MenuHomePhabricator

Gerritbot shouldn't post "Change <x> merged by jenkins-bot:" messages any more
Closed, DeclinedPublic

Description

Phabricator now tells us; this is just spam.

Event Timeline

Jdforrester-WMF raised the priority of this task from to Needs Triage.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

I have a feeling I've asked this before, but - do we have those messages from Phabricator on *all* repositories now?

I have a feeling I've asked this before, but - do we have those messages from Phabricator on *all* repositories now?

Phabricator/Diffusion/Callsign naming conventions/Existing repositories. I don't know if this list is accurate.

I've seen Phab add '[user] mentioned this in <commit id>' , but it isnt clear that it is merged. Is that a pending task?

chasemp added a subscriber: chasemp.

Can someone link to a comment of the type referenced here?

I have a feeling I've asked this before, but - do we have those messages from Phabricator on *all* repositories now?

All but about 100 remaining ones (out of > 1200), and none of those are the big ones.

Phabricator only tells you about merged commits that mention a task somewhere in the commit message.
But at some point we decided that we only care to leave comments when the task has been mentioned it the
commit message's last paragraph via Bug: $TASK_ID.

See https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines#Example

So gerritbot's notifications differ from the phabricator notifications.

Changing and telling gerritbot to no longer comment on merges is simple.

Hence ... is there some rough consensus that we want to change?

(I for one care care way more about commits that really address a task (i.e.: Bug: $TASK_ID in last paragraph) than those that just mention a task somewhere)

I have a feeling I've asked this before, but - do we have those messages from Phabricator on *all* repositories now?

All but about 100 remaining ones (out of > 1200), and none of those are the big ones.

I dont think I am seeing these messages on the pywikibot tasks, which I believe is one of the more active repositories.

I have a feeling I've asked this before, but - do we have those messages from Phabricator on *all* repositories now?

All but about 100 remaining ones (out of > 1200), and none of those are the big ones.

I dont think I am seeing these messages on the pywikibot tasks, which I believe is one of the more active repositories.

Ah, I've found a few have come through in emails like T93192. We might not be seeing them all because some of our commit messages still have bugzilla IDs, like rPWBCad9094604c45 , which I have retrospectively linked to T68897.

Phabricator only tells you about merged commits that mention a task somewhere in the commit message.
But at some point we decided that we only care to leave comments when the task has been mentioned it the
commit message's last paragraph via Bug: $TASK_ID.

See https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines#Example

So gerritbot's notifications differ from the phabricator notifications.

Changing and telling gerritbot to no longer comment on merges is simple.

Hence ... is there some rough consensus that we want to change?

(I for one care care way more about commits that really address a task (i.e.: Bug: $TASK_ID in last paragraph) than those that just mention a task somewhere)

I believe this restriction was put in place because the gerritbot (at the time for Bugzilla) did more than just leave a comment. It also changed the bug status. This is still the case in the incarnation for Phabricator as well (it adds Patch-For-Review). Mentioning that it was mentioned in itself seems valuable and either way is unavailable in Phabricator as that its nature (which I think is a good thing to have).

Indicating whether a commit fixes a bug is done in Phabricator with the phrase Fixes T123 (generally as last line of the commit message body before the meta footer). Which even auto-closes the task when the commit is merged (just like on GitHub).

auto-closing a task on merge assumes that all intended fixes are actually working (without verifying them). in reality they often need follow-ups and we should have the habit to confirm fixes after merging. it's like in Bugzilla you have "resolved" but it's not the last workflow step, it's only somebody claiming resolved and the last step is actually "verified".

Can someone link to a comment of the type referenced here?

could you also link to an example that would replace this? Since the ticket says phabricator now tells us but i haven't really seen that. At least for ops/puppet these bot messages have been really useful.

could you also link to an example that would replace this? Since the ticket says phabricator now tells us but i haven't really seen that. At least for ops/puppet these bot messages have been really useful.

They should be right below the gerritbot comment.

https://phabricator.wikimedia.org/T93151#1146049

@Dzahn mentioned this in rOPUP252847e946dc: make twentyafterfour a phabricator-admin

https://phabricator.wikimedia.org/T93261#1145720

@Dzahn mentioned this in rOPUPd72a449559e4: add ferm service for poolcounterd
@Dzahn mentioned this in rOPUP94ba37e61e50: have base::firewall on codfw poolcounters

Example of Fixes T...:
https://gerrit.wikimedia.org/r/193330
https://phabricator.wikimedia.org/T85895#1072911

Mattflaschen closed this task as "Resolved" by committing rEFLW001bd62e578b: Add i18n for flow-create-board right

Ok, thanks. But mentioning a bug in a commit message just means it got uploaded for review. The other interesting part is when or _if_ it actually gets merged. I think the bot messages are actually better. There is one comment for the initial upload and one when/if it gets merged (but not one for each new patch set in the same change because that would be too much) and i like that. "auto-resolve by committing" feels like it lacks verification as mentioned above.

hashar added a subscriber: hashar.

Removing Continuous-Integration-Infrastructure since this is about the Gerritbot that relay Gerrit event to Phabricator. We have enough bugs as is.

That being said, I like the notification on tasks, that let me know something happened on that task and take action such as closing the task. Not all repos are in Diffusion nor do Diffusion always notify me of a patch.

@QChris mentioned gerrit bot only reports on tasks explicitly mentioned via a bug: Txxx field whereas Phabricator does it for any Txxx occurrence.

@Dzahn and @hashar wants the notification when a change is merged in Gerrit.

Seems to me there is no consensus to disable it. I am thus closing this task for now. One can reach out to more people to figure out a nice solution to reduce notification spam.