Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 384129 "Revert Patchset" button on codereview.chromium.org does not add TBR and NOTRY to the revert patch
Starred by 0 users Project Member Reported by rous...@chromium.org, Jun 12, 2014 Back to list
Status: WontFix
Owner: jrobb...@chromium.org
Closed: Jun 2014
Cc: acolwell@chromium.org, sorin@chromium.org, kalman@chromium.org, jrobb...@chromium.org, pgervais@chromium.org, binjin@chromium.org
Components:
OS: ----
Pri: 2
Type: Bug


Sign in to add a comment
Steps to reproduce:
1) Find a patch you want to revert on https://codereview.chromium.org/.
2) Click on the "Revert Patchset" button.

Expected:
The reverting patch has TBR=<original-patch-owner> and NOTRY=true. The CQ commits the reverting patch.

Observed:
The reverting patch does not have TBR and NOTRY. The CQ attempts to commit the reverting patch, but cannot, because of no LGTM from an owner.

------
See https://codereview.chromium.org/332523008/ for example. This started happening today (Jun 12, 2014) and happened for both patches that I attempted to revert.

Workaround: Manually edit the description of the reverting patch to add the NOTRY and TBR.
 
Comment 1 by pgervais@chromium.org, Jun 12, 2014
Cc: jrobb...@chromium.org
+jrobbins who is in charge of codereview.chromium.org
Comment 2 by sorin@chromium.org, Jun 13, 2014
Thank you for letting us know? Is there a reason to revert a revision using the CQ? I've been using drover to revert it, for some reason CQ had never been instantaneous in the past, even when revert patch set worked. 
Comment 3 by rous...@chromium.org, Jun 13, 2014
Seems to be fixed now. Not sure what fixed it.
Comment 4 by jrobb...@google.com, Jun 13, 2014
Owner: jrobb...@chromium.org
Status: WontFix
There is actually logic in Rietveld that conditionally adds the TBR no NOTRY only for recent patches.  The reason is that a concern was raised about people being able to revert only changes on the sly.  So, what you saw was working as intended, but I can see how it could be confusing.
Sign in to add a comment