Right now, gaia.json updates to b2g-inbound from the Gaia pushbot show up as an unholy combination of merge commits and regular commits when running mcMerge. In the vast majority of cases, we really don't want to comment in the bugs at all for Pushbot commits. We should treat Pushbot commits as another step in mcMerge similar to merges, backouts, etc rather than lumping them in with the rest. Example Pushbot commit: https://hg.mozilla.org/integration/b2g-inbound/rev/59417b7613c7

Wes/Tomcat, up for taking a look? :-) I think the simplest fix here (or at least one that we'd want in addition anyway), is just to make mcMerge only look at the first line of the commit message, when looking for bug numbers. That way the gaia robot messages will end up in the "other" category (that is excluded from bug marking), which is what we want :-) We should only save the first line of the commit message in push.desc, here: https://hg.mozilla.org/webtools/tbpl/file/b21b0fb97737/mcmerge/js/PushData.js#l425 To test, run mcmerge/index.html from the local filesystem.

taking the bug

Give a shout if you need any help with this :-)

Tomcat, any luck with this? :-)

(In reply to Ed Morley (Away at DashCon; back on 27th Jan) [:edmorley UTC+0] from comment #4) > Tomcat, any luck with this? :-) hey ed, not so far but still planning to fix this :)

(In reply to Carsten Book [:Tomcat] from comment #5) > (In reply to Ed Morley (Away at DashCon; back on 27th Jan) [:edmorley UTC+0] > from comment #4) > > Tomcat, any luck with this? :-) > > hey ed, not so far but still planning to fix this :) Tomcat, are you still interested in doing this, or should I make it a good first bug? (just seeing if I can make a few, given dburns email :-))

yeah this should make a great first bug, thanks for the suggestion ed

Only use the first line of the commit message, to avoid false positives when checking for bug numbers and backouts later. This prevents B2G bumper bot commits from being detected as a bug in need of marking due to the bug numbers in the extended commit message.

Apologies for delay Ed: aiming to get to this review on Wednesday 13th.

Comment on attachment 8466269 [details] [diff] [review] Only use the first line of commit messages Review of attachment 8466269 [details] [diff] [review]: ----------------------------------------------------------------- I looked over some truncated output from this unholy command: hg log -v --template "\nchangeset: {node|short}, first line: {desc|firstline}" | grep -vE "[[:space:]=][[:digit:]]{6,8}([[:space:].:;,)]|$)" | grep -Ev "(gaia|b2g)-bump" | grep -Evi "merge m-(c|i)|b-i|b2g(-i)?|f-t|back ?out|mozilla-(central|inbound)|fx-team|(b2g-)?inbound" | grep -Evi "[ (]no bug([ ).;:,]|$)" | grep -Evi "bumping gaia.json" > commits.txt which was intended to catch any commits with nothing bug-number-like in the first line. I went back until I was hitting commits from 2011. Most were—as expected—merges and backouts, but some were changesets where this approach would have truncated off the bug number(s). Admittedly they represented a tiny fraction of commits, but I found: https://hg.mozilla.org/mozilla-central/rev/29dd2ddfdf8b https://hg.mozilla.org/mozilla-central/rev/34de49884318 https://hg.mozilla.org/mozilla-central/rev/4f9563af1fa1 https://hg.mozilla.org/mozilla-central/rev/7b40032d4d18 https://hg.mozilla.org/mozilla-central/rev/755f7983b4e2 https://hg.mozilla.org/mozilla-central/rev/30bd1b29ac64 and I've likely missed some. I think I'd prefer Ryan's suggestion in comment 0: break BumperBot commits out into a separate step. The following should be sufficient: 1) Add a property to Config.js containing a string representing BumperBot's email (or possibly an array containing the one string if we think there might be other emails we wish to filter on in future). 2) Add appropriate text to the help text and headings data in Step.js (https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/mcmerge/js/Step.js#l1038) 3) Add an entry (using the same property name as in step 2) to the stageTypes array in mcMerge,js (https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/mcmerge/js/mcMerge.js#l12). Note the order of this array dictates the order in which the steps appear. 4) Add a new empty array—again with the same name—to the declarations at the top of PushData.js (https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/mcmerge/js/PushData.js#l7). (Optional: you could go ahead and delete the clear function just below, as it's entirely redundant). 5) Wrap lines 438-446 of PushData.js in a conditional that only executes if push.email !== bumper bot's email 6) Add a filter in classifyPushes (https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/mcmerge/js/PushData.js#l480) that adds the push to the array from step 4 if the email matches (or you could add a boolean isBumperBot property to the push object in the else leg of the conditional in step 5 and test for that) 7) Add a line to reverse the array at (https://hg.mozilla.org/webtools/tbpl/file/06af9dad7e20/mcmerge/js/PushData.js#l509) (PushData builds its arrays backwards). A bit more work, yes, but still fairly mechanical. Note to self: the step stuff is spread out over PushData/Step/mcMerge: that's horrible. "Steps" should be considered a piece of data and centralised in Config.js with their names, text, heading and filter function.

Comment on attachment 8466269 [details] [diff] [review] Only use the first line of commit messages The commits mentioned in comment 10 are not valid IMO; that represents a bug in the commit message hook (we're tweaking it in bug 1053002, I'll either get them to fix it there or file another bug). Even if we special-cased the B2G bumper bot, there are still false positives caused by use using content not in the first line of the commit message. I don't have a specific example commit to hand right now, but I've seen instances where "revert" or similar are used on lines 2+ and falsely caused the commit to be included in the "things that were backed out but did not land in this push" list. As such, I think we should still go with the approach in this bug.

(In reply to Ed Morley [:edmorley] from comment #11) > Even if we special-cased the B2G bumper bot, there are still false positives > caused by use using content not in the first line of the commit message. In commits made by people, not the bot.

Good research on finding those commits though :-) (And thank you for listing all the steps for special-casing out!)

I've filed bug 1053177 for enforcing that the commit message hook is in the first line.

Graeme, given comments 11-14, what are your thoughts? Even if we decide to add the ability to special-case certain push authors in the future, I think limiting mcMerge to the first line of the commit message is still the correct thing to do (given the potential for !commit-bot false positives, and the hghook change to enforce correct firstlines) - and will solve this bug in a one line change as an added bonus :-)

Comment on attachment 8466269 [details] [diff] [review] Only use the first line of commit messages We could really do with this fix, and bug 1053177 which alleviates the issues in comment 10 is now fixed.

Comment on attachment 8466269 [details] [diff] [review] Only use the first line of commit messages Review of attachment 8466269 [details] [diff] [review]: ----------------------------------------------------------------- I think matching on the first line only is more consistent with our other tools, and that's a good thing.

Comment on attachment 8466269 [details] [diff] [review] Only use the first line of commit messages remote: https://hg.mozilla.org/webtools/tbpl/rev/9cb4d4589a82