No message sent to sheriffs for reverts created on Gerrit |
||
Issue descriptionUrl to the build Failure: https://luci-milo.appspot.com/buildbot/chromium.linux/Android Builder/83158 What is the bug or feature: Like Rietveld, we should send a message to sheriffs after adding them as reviewers of the reverts. And the culprit CL itself, we should post a message on why we revert it like what Findit does for Rietveld.
,
May 23 2017
I'm still wondering whether there is equivalent API to add reviewers and post message at the same time, as the "Reply" button on the UI.
,
May 23 2017
If this is a bug in the gerrit.py, please assign to robertocn@. If the change is needed on the pipeline layer, chanli@ should still take it. Anyway, collaboration is needed here.
,
May 24 2017
Different from rietveld, each time when we add reviewers and a message, then hit 'Send' button in 'Reply' dialog, gerrit will post separated messages for adding reviewers and adding message. I think that means we have to send message and add reviewers separately. I can make the change at pipeline side since we need to send separated requests anyway.
,
May 24 2017
Architecturall, I'd prefer to make gerrit and rietveld api's as similar as possible so that we wouldn't need to make separate code paths in the pipeline. So making the addreviewers also send a message for gerrit doesn't seem too bad. I'll leave up to Chan, either way works and it's not too big a difference between the two options.
,
May 24 2017
On second thought, to make it consistent with rietveld, the change should be at gerrit side. But I'll still do it since the change is small.
,
May 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/6e402e015a6cde16b7ecb1e09bc2c221e70c331e commit 6e402e015a6cde16b7ecb1e09bc2c221e70c331e Author: Chan <chanli@chromium.org> Date: Fri May 26 01:00:26 2017 Post reason of revert when revert gerrit CLs. Post a message when adding reviewers. Also remove assertion forexistance of sheriffs when adding reviewers. Bug: 725628 Change-Id: Iadc80c511b3f71627b44b3dc4ae4b801fcbd8744 Reviewed-on: https://chromium-review.googlesource.com/514509 Commit-Queue: Chan Li <chanli@chromium.org> Reviewed-by: Roberto Carrillo <robertocn@chromium.org> Reviewed-by: Shuotao Gao <stgao@chromium.org> [modify] https://crrev.com/6e402e015a6cde16b7ecb1e09bc2c221e70c331e/appengine/findit/waterfall/create_revert_cl_pipeline.py [modify] https://crrev.com/6e402e015a6cde16b7ecb1e09bc2c221e70c331e/appengine/findit/infra_api_clients/codereview/gerrit.py [modify] https://crrev.com/6e402e015a6cde16b7ecb1e09bc2c221e70c331e/appengine/findit/infra_api_clients/codereview/test/gerrit_test.py
,
May 26 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by robert...@chromium.org
, May 23 2017