Last modified: 2013-04-22 11:01:25 UTC
Created attachment 11883 [details] gerrit mediawiki bug reference Per the wikitech thread, gerrit should learn that we like the bug on the top, not on the bottom. The (untested) attachment grabs the bug number from the short message so that its bug: search can find it. We should test in gerrit-dev.wmflabs.org and check it works correctly.
Some of your lines seem to be differently indented
Should be submitted upstream (upstream bug for reference: https://code.google.com/p/gerrit/issues/detail?id=849). We can try a custom build on gerrit-dev if we really want, but I won't deploy custom builds to WMF again :\
I tried to follow the indenting convention of the surrounding code, and seems to be doing that right. (They apparently use two-spaces for full lines and four-spaces for continued lines) ^demon, I would appreciate a confirmation that it at least works, I can try doing it myselft, with a few directions. However, I'm afraid that as provided it's too specific to be accepted upstream. Would a gerrit plugin be able to do this? I have no idea how they are implemented internally, but adding the needed hooks for a custom plugin which detects the Bug # will probably be more easily accepted upstream.
As there are not too many extension points yet, plugins cannot be hooked in at the relevant places to help with this problem. However, I just tried to apply the patch locally, but it does not apply cleanly any longer. Taking a look nonetheless, I guess there might be a problem with generating the FooterLine. In the patched line 135 in ChangeUtil.java, it seems the FooterLine constructor is missing around the arguments of add. However, that constructor would have package visibility anyways, so we cannot simply call it from within gerrit. We'd have to shoehorn the "subject's tracking id" in a different way. You could rearrange the nested for loops of updateTrackingIds, refactor the inner body into a private method, and use this method to pass the "subject's tracking id". But that's tedious. On top of that I share your fear that hard-coded checking for "(bug ...)" might be too specific to be accepted upstream. However, the upstream bug that Chad gave looks like a perfect match. One can build on that and try to parse a commit's subject for arbitrary tracking id's and shoehorn them in. That should meet our problem, the upstream bug, and it would be general enough to have a chance of being accepted upstream.
Can we please just settle on a format (be it the top in parenthesis or the bottom as a footer). I really can't believe there's been bikeshedding on this on the mailinglist, and then to end up some of the people involved implementing a patch to give the old format some of the advantages the new code has? That doesn't solve anything. We're going to enforce one format. One format to recommend to people to use in writing commit messages. One format for Gerrit to extract bug references from for searching. One format for wmf-branch maker to render notes from. One format for gerrit-bugzilla bot (if/when that happens) to extract references from. Supporting both is *not* an option. It just isn't. It only makes things harder to maintain (duplicating logic everywhere for everything related to commit messages, twice). And aside from maintaining code and spending time on that, it is also costing towards reviewers. Sometimes having those <bias> useless unclickable numbers show up in the title bar and e-mail headlines, and sometimes not.</bias> Sometimes opening it up and looking down for the link, and sometimes looking up for the link. I think the footer is where this information belongs, but whatever. Doing both is *not* an option.
> and then to end up some of the people involved implementing a patch to give > the old format some of the advantages the new code has? That doesn't solve > anything. If the problem with the “old format”, as you call it, is that gerrit doesn't support searching it, the solution is to implement that in gerrit, not to change the format.
(In reply to comment #6) > > and then to end up some of the people involved implementing a patch to give > > the old format some of the advantages the new code has? That doesn't solve > > anything. > > If the problem with the “old format”, as you call it, is that gerrit doesn't > support searching it, the solution is to implement that in gerrit, not to > change the format. We didn't switch from "(bug #)" to "Bug: #" because that's the only way Gerrit supports it. We switched it also for various other reasons: 1) More space in the change subject 2) Reduce irrelevant information from relevant context In cases were there is only a change subject and not the full commit message (gerrit search results, git log, email subject, irc notifications) the "bug .." is not linked but plain text. Since a number on itself is generally not useful that's only in the way. In cases where you can click it (gerrit change page) the full message is shown so it doesn't matter where it is. 3) Ability to distinguish between mentioning a bug (and getting it linkified) and associating it with the change as metadata. Especially now that we have a Gerrit-Bot for Bugzilla that leaves a comment when a change is associated with it. So that when a change says something like "This isn't foo (bug 123), but something else" it doesn't trigger the index. It is meta data, it belongs in the footer. There is bias and personal preferences and we'll have to get used to these minor changes in our workflow but afaik there are no significant advantages to putting it in parentheses on top. And all of the above to doing it in the footer as correctly identified meta data. Marking it as wontfix per reason #3. I noticed this earlier today where a change mentioned various bug Ids in the commit message. If all those lead to internal index associations with bugs it would've resulted in a mess. Also note that "bug #" is already recognised as a link, just not as meta data. And that's intentional.
