Page MenuHomePhabricator

gerritbot should listen for "Task:" as well as "Bug:" in Gerrit commit messages
Closed, DeclinedPublic

Description

I think @gerritbot should listen for "Task:" as well as "Bug:" in Gerrit commit messages.

Event Timeline

MZMcBride raised the priority of this task from to Needs Triage.
MZMcBride updated the task description. (Show Details)
MZMcBride added a project: Gerrit.
MZMcBride added subscribers: MZMcBride, gerritbot.
Aklapper triaged this task as Lowest priority.Feb 27 2015, 12:32 PM

Copying from T91051:

If writing "Bug: " as a prefix feels too weird, feel free to write
support for more string prefixes via patching
src/main/java/com/googlesource/gerrit/plugins/hooks/bz/InitBugzilla.java
in operations/gerrit/plugins/its-phabricator-from-bugzilla.jar (I think,
qchris was kind enough to set that part up) &&
operations/puppet/modules/gerrit/files/its/action.config (for Gerritbot
comments in Phabricator). Also needs changes to the regex in
operations/puppet/modules/gerrit/templates/gerrit.config.erb (for
auto-linking the prefix in the Gerrit web UI).

Plus documentation update in https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines#Body

Could we also do an agnostic prefix like "#"?

Example:
#T91051

@Aklapper, thanks for copying that really helpful note! It would be nice if we could centralize documentation about Gerrit bots; I eventually found https://wikitech.wikimedia.org/wiki/Gerrit_Notification_Bot.

I'm not sure why you removed the good first task tag. (Intentional?)

Yeah, removing good first task was intentional - I don't consider this trivial enough, seeing the amount of programming languages and places to fix involved. :-/

Double thumbs up to agnostic prefix. I miss this about github.

Change 302482 had a related patch set uploaded (by Paladox):
Gerrit: Support footer prefix Task: for phabricator

https://gerrit.wikimedia.org/r/302482

I'm not a big fan of this, as it requires every other tool that parses Bug: lines (e.g. ReleaseTaggerBot) to also parse all alternative forms. Having one standard form (where it's also clear that other forms are incorrect because they are not linked) is IMO preferable.

What @valhallasw said. Our guidelines say to use "Bug:", and we've standardized on that. Adding Task support will just make it inconsistent :(

The problem I see is, that since we have phabricator, we don't have "Bugs", we have "Tasks", which is more useful (in my opinion), for example a config change for a community is not a bug, but a request, so generally it's a task. I think it would be at least better, if other tools support "Task" too.

The problem I see is, that since we have phabricator, we don't have "Bugs", we have "Tasks", which is more useful (in my opinion), for example a config change for a community is not a bug, but a request, so generally it's a task. I think it would be at least better, if other tools support "Task" too.

We could argue over the dictionary definitions of bug and task, but that's not really my point. My point is to have a standard, and stick to it. If people overwhelmingly prefer Task, I'm okay with that, but as long as we *only* use "Task". Supporting multiple formats is what I don't want to do, because that's inconsistent and becomes a pain for tools and bots. And when you realize exactly how much work is needed to switch from Bug to Task, you'll quickly realize that it's not worth it.

I don't think we ever made guarantees to third-party tools/clients/consumers regarding commit messages, so I'm less concerned with those.

I also don't think it would be a lot of work to switch from supporting "Bug: " to supporting "Task: ". There would likely be more work to support both.

That all said, the level of effort required to make this change does seem to pretty heavily outweigh the benefit it would provide. We have a commit message standard, even if it's dumb, and sticking to using "Bug: " might be best.

demon added a subscriber: demon.

Declining per all of the above. Bug, in retrospect, is a terrible name. But supporting dueling standards is even more terrible :)

Change 302482 abandoned by Chad:
Gerrit: Support footer prefix Task: for phabricator

Reason:
Because the associated task is declined.

https://gerrit.wikimedia.org/r/302482