Try bot emails for successful runs are unnecessarily cluttered |
|||||
Issue description
Here's that the body of commit bot emails for a successful trybot run:
```
Patch set 1:
Dry run: This issue passed the CQ dry run.
Bot data: {"action": "cancel", "triggered_at": "2017-04-24T20:54:27.0Z", "revision": "554dfd25103a2185ca6d2cdd5946aac03406ce6b"}
```
I would have been fine with something like:
```
Patch set 1:
This issue passed the CQ dry run.
```
The "Dry run:" prefix seems redundant. And the "Bot data:" section looks like something that would be useful only to people debugging some CQ or Gerrit issue. To me, it looks like noise and involves a cognitive overhead every time I get one of these messages: it takes me a second or two to realize that everything went fine, and this isn't some cryptic error.
For bonus points, I'd love if the "Passed the dry run" message could make it into the Gmail snippet, though this probably isn't possible since the message always begins with "Commit Bot posted comments on this change" (should I open a separate bug for that? this seems redundant, since the From: header already contains "Commit Bot (Gerrit)").
,
Apr 25 2017
,
Jun 6 2017
+1. In particular, the "cancel" action is really confusing -- did this pass or not?! Also, @logan: what is the rationale for a gerrit bug being located in the gerrit project vs chromium project?
,
Jun 6 2017
@adamk: Yes, we know. Unfortunately, it is necessary for now, due to the stateless nature of the commit queue. If it loses its state and has to reconstruct it, it does so from those pieces of bot data. We plan to change that in the future, but don't have a way to do so immediately. If you have suggestions for how to make the data less distracting, I'm all ears. @tandrii: What are the chances we can safely remove this? Is the CQ stable enough that it isn't necessary anymore? @michaelpg: It depends on who would be responsible for fixing it; the commit queue is owned by Chrome Infra, so those bugs get moved into our component of /p/chromium.
,
Jun 6 2017
@agable: you're saying it uses this data in the email message to reconstruct that state? My suggestion would be to put this where there is space for metadata (if it were actually email, a header). But I'm guessing Gerrit doesn't have a place for metadata in messages?
,
Jun 6 2017
so yes, gerrit doesn't have a place for metadata. and actually yes, CQ can avoid adding "bot data" to cancellation message iff CQ removed CQ+1 messages successfully prior to that. If you can split other parts of this bug into new bugs, then i'd keep this bug in CQ component.
,
Aug 3 2017
Any progress on this? I still do a double-take every time I get a CQ message.
,
Aug 7 2017
adamk@ sorry, none yet. I might look at this once I sweep across many CQ Pri2 bugs at once, but I'm currently 100% on a different project :(
,
Aug 21 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/infra_internal/+/716ece7232c34f9cd510ebdee4aa83f8262fa93d commit 716ece7232c34f9cd510ebdee4aa83f8262fa93d Author: Andrii Shyshkalov <tandrii@chromium.org> Date: Mon Aug 21 07:57:12 2017
,
Aug 21 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infra/puppet/+/7002f99e5d6e99db034b4264e7265857a3fb00f3 commit 7002f99e5d6e99db034b4264e7265857a3fb00f3 Author: Andrii Shyshkalov <tandrii@chromium.org> Date: Mon Aug 21 11:20:21 2017
,
Aug 21 2017
So, I had looked into CQ code today and removed "bot data" from final messages posted by all CQ Dry Runs + failed Full CQ runs*. If you still see "Bot data: ...." in your emails, please file a new bug. * except when CQ failed to remove triggering CQ vote, which happens either due to misconfiguration of ACLs OR rarely due to flakes in network and/or gerrit. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by logan@google.com
, Apr 25 2017Labels: Proj-Gerrit-Migration