New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 875886 link

Starred by 2 users
Status: WontFix
Owner: ----
Closed: Aug 20
Cc:
agable@google.com
█ dpranke@google.com
ehmaldonado@chromium.org
rmis...@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Adding OWNERS files prevent reverts

Project Member Reported by fhorschig@chromium.org, Aug 20 
When trying to land https://crrev.com/c/1181101, the rule "No TBR if OWNER files are affected" prevented a critical revert for a large change.

For cases like this, where we revert adding OWNERS, it might make sense make exceptions of that rule.


(The CL to be reverted changed ~30k lines of code and had 19 reviewers. It broke several Android tests and this made the difficult revert even harder.)

Comment 1 by thakis@chromium.org, Aug 20

Cc: ehmaldonado@chromium.org dpranke@google.com agable@google.com
Actual text: "The CL affects an OWNERS file, so TBR will be ignored"

That behavior is from https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/982601 . The author is away, so cc'ing reviewers too.

Being unable to revert CLs that touch OWNERS files seems like an unintentional side effect of that change.

Comment 2 by dpranke@chromium.org, Aug 20

Status: WontFix (was: Untriaged)
The description in the depot_tools change talks about how you can bypass the check for critical reverts (by using NOPRESUBMIT=true). 

The change was otherwise intentional, and I think things worked as they should've.

Please reopen if I'm missing something.

Comment 3 by thakis@chromium.org, Sep 22 

Another example: https://chromium-review.googlesource.com/c/chromium/src/+/1239021 touched one file in content/ in a minor way and deleted a bunch of code in chrome/ (which had lgtm). The TBR for content/ failed because an OWNERS file in a folder that didn't need tbr was deleted.
►

Sign in to add a comment