New issue
Advanced search Search tips

Issue 744703 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Suggested OWNERS shows irrelevant "Best not to use globs..." comment.

Project Member Reported by lgar...@chromium.org, Jul 17 2017

Issue description

Chrome Version: f535f3e8476a4727232607d5d887409686405f45

What steps will reproduce the problem?
(1) Upload a patch like https://chromium-review.googlesource.com/c/574718/ with `git cl upload`

What is the expected result?
Useful stuff. Nothing confusing.

What happens instead?

    Suggested OWNERS: (Use "git-cl owners" to interactively select owners.)
        dgozman@chromium.org is commented as:
          Changes to remote debugging protocol require devtools review to
    ensure backwards compatibility and committment to maintain. (at third_party/WebKit/Source/core/inspector/browser_protocol.json)
        estark@chromium.org is commented as:
          Note: Best not to use globs (like autofill*) due to  crbug.com/397984  (at components/page_info_strings.grdp)

The last line ("Best not to use globs...") is from https://cs.chromium.org/chromium/src/components/OWNERS?dr&q=%22Best+not+to+use+globs%22&sq=package:chromium&l=6

I feel like I've seen this a whole bunch, and I was confused the first few times. It would be better if the heuristics didn't attach this comment with a very indirect association to the reviewer recommendation, or there was a mechanism to ensure that such comments are not accidentally present in a way that they will e associated like this (unless intended).
 

Comment 1 by aga...@chromium.org, Jul 17 2017

The easy solution is to edit each OWNERS file to make sure that there aren't such (unintended) comments immediately before a list of owners. You only need to look at 275 files with comments:

± git grep -w -e '^#' '**/OWNERS' | grep -v TEAM | grep -v COMPONENT | cut -d ':' -f 1 | uniq | wc -l
275
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4e766b8e0aec9743d1fdc47a3134abdf41a976d

commit d4e766b8e0aec9743d1fdc47a3134abdf41a976d
Author: Lucas Garron <lgarron@chromium.org>
Date: Thu Aug 03 16:32:29 2017

Prevent components/OWNERS comment from being spuriously associated by presubmit.

Bug:  744703 
Change-Id: Ibbded8b3ea454856543802eacc46cab8ff766cfc
Reviewed-on: https://chromium-review.googlesource.com/599009
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491763}
[modify] https://crrev.com/d4e766b8e0aec9743d1fdc47a3134abdf41a976d/components/OWNERS

Status: Fixed (was: Untriaged)
I'll mark this as fixed and send more CLs if I see this again.

Sign in to add a comment