Bug 38936 - sheriffbot is spammy.
: sheriffbot is spammy.
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-05-11 14:39 PST by
Modified: 2010-05-11 21:37 PST (History)


Attachments
Patch (8.61 KB, patch)
2010-05-11 18:20 PST, Adam Barth
eric: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-05-11 14:39:16 PST
Today the tree went red.
Sherriffbot decided to blame everyone for the failure except for the right change which was 59171.

sheriffbot: dave_levin, ojan: http://trac.webkit.org/changeset/59168 might have broken Chromium Win Release
[2:25pm] sheriffbot: abarth, japhet: http://trac.webkit.org/changeset/59169 might have broken Chromium Win Release
[2:25pm] sheriffbot: ggaren: http://trac.webkit.org/changeset/59170 might have broken Chromium Win Release
[2:25pm] sheriffbot: tonikitoo: http://trac.webkit.org/changeset/59165 might have broken Chromium Win Release
[2:25pm] sheriffbot: jianli: http://trac.webkit.org/changeset/59166 might have broken Chromium Win Release

A few possible issues: 
1. Perhaps it would be better to do more of a summary.
2. It is odd that it omitted the right change.
3. Does it keep telling people that they may have broken the build if the build was already broken and didn't turn green yet?
------- Comment #1 From 2010-05-11 16:41:34 PST -------
The problem is that SheriffBot detected that 59171 broke Qt Linux Release minimal already.  When Chromium Win Release broke, the blamelist included a bunch of other patches.  In an effort not to spam, it didn't repeat the warning for 59171.

One proposal is to not warn about a blamelist that has some intersection with an already blamed list.  That will reduce false positives in cases like this at the cost of missing some warnings about concurrent failures.
------- Comment #2 From 2010-05-11 18:20:10 PST -------
Created an attachment (id=55792) [details]
Patch
------- Comment #3 From 2010-05-11 20:54:28 PST -------
(From update of attachment 55792 [details])
WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py:64
 +          # been that skippign this check causes a lot of spam for builders that
skippign

Makes sense.

I still think our sheriffbot arch is less than optimal.  I would like him eventually to hold in memory and increasingly good picture of what went wrong.  So that one can ask him "what's broken" and he can tell you the exact revision and what it broke.  Right now he just warns as he gets information and doesn't keep around any bigger picture.

Ideally we should cross-reference builds to figure out what actually started when and hold multiple "break" objects or something in memory.  Then we would warn about them every 30m or something until they got fixed, and could explain all the information we know about them over IRC when asked.
------- Comment #4 From 2010-05-11 21:20:20 PST -------
Committed r59204: <http://trac.webkit.org/changeset/59204>
------- Comment #5 From 2010-05-11 21:37:01 PST -------
Committed r59206: <http://trac.webkit.org/changeset/59206>