Issue 1493956 in chromium: Stop assuming @chromium.org for flag-metadata.json owners

[Git Watcher via monorail]

Updates:
Status: Fixed

Comment #4 on issue 1493956 by Git Watcher: Stop assuming @chromium.org for flag-metadata.json owners
https://bugs.chromium.org/p/chromium/issues/detail?id=1493956#c4

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

commit 4bd760facd0c49546c84d26ad7a48684287f1e74
Author: Victor Hugo Vianna Silva <victor...@google.com>
Date: Thu Oct 19 14:38:03 2023

Stop assuming @chromium.org for flag-metadata.json owners

An owner without @ and without /OWNERS is currently interpreted as a
@chromium.org email. However, contributors with @google.com emails are
mistakenly omitting the domain, causing them to not get nags about
expired flags. See crrev.com/c/4953374 for one such case. Most new
Google contributors use @google.com emails and are prone to this mistake.

This CL:
- Drops support for such syntax in EnsureOwnersLookValid(). Trying to
use it will cause a failure of AboutFlagsTest.OwnersLookValid.
- Updates flag-metadata.json to use @chromium.org where appropriate
with a script, so the test passes.
- Updates the documentation on top of flag-metadata.json.

Fixed: 1493956
Change-Id: I259cd62705a0bd74351c7631285d966dd7952dfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4952502
Reviewed-by: Elly FJ <elly...@chromium.org>
Commit-Queue: Victor Vianna <victor...@google.com>
Reviewed-by: Avi Drissman <a...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212140}

[modify] https://crrev.com/4bd760facd0c49546c84d26ad7a48684287f1e74/chrome/browser/flag-metadata.json
[modify] https://crrev.com/4bd760facd0c49546c84d26ad7a48684287f1e74/components/flags_ui/flags_test_helpers.cc

--
You received this message because:
1. You are auto-CC'd on all issues in component Internals>Flags

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment or make updates.

[Git Watcher via monorail]

Comment #5 on issue 1493956 by Git Watcher: Stop assuming @chromium.org for flag-metadata.json owners
https://bugs.chromium.org/p/chromium/issues/detail?id=1493956#c5

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src/+/567d88c1b7c68bb10b4c9dfb01c43699bd634ede

commit 567d88c1b7c68bb10b4c9dfb01c43699bd634ede

Author: Victor Hugo Vianna Silva <victor...@google.com>

Date: Thu Oct 19 15:37:17 2023

Fix AboutFlagsTest.OwnersLookValid

A new entry was added to flags-metadata.json before crrev.com/c/4952502
was submitted and CQ leaned on the successful test run, so the test
now fails on CI.

Land this one with No-Presubmit/ No-Tree-Checks / No-Try because
the longer we take, the more risk we have the JSON is updated again.
Also, changing this file can't break the build, so we're good.

Bug: 1493956
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I1bd4bab9d5853e9a1bf0a33637decfbaf49161f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4950125
Auto-Submit: Victor Vianna <victor...@google.com>
Reviewed-by: Yi Gu <yi...@chromium.org>
Commit-Queue: Yi Gu <yi...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212179}

[modify] https://crrev.com/567d88c1b7c68bb10b4c9dfb01c43699bd634ede/chrome/browser/flag-metadata.json

[Git Watcher via monorail]

Comment #6 on issue 1493956 by Git Watcher: Stop assuming @chromium.org for flag-metadata.json owners
https://bugs.chromium.org/p/chromium/issues/detail?id=1493956#c6

The following revision refers to this bug:

https://chromium.googlesource.com/chromium/src/+/31057862f6e868237b1464eee79c655c1d545706

commit 31057862f6e868237b1464eee79c655c1d545706
Author: Alexander Bolodurin <ale...@chromium.org>
Date: Fri Oct 20 05:33:28 2023

Fix AboutFlagsTest.OwnersLookValid

Fix an invalid entry in flag-metadata.json. Implicit @chromium.org was
removed in https://crrev.com/c/4952502.

Bug: 1493956
Change-Id: I3e2acb3fb96134f46718925a4305bd45b2a5968d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956035
Reviewed-by: Justin Cohen <justi...@chromium.org>
Owners-Override: Justin Cohen <justi...@chromium.org>
Commit-Queue: Alexander Bolodurin <ale...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212581}

[modify] https://crrev.com/31057862f6e868237b1464eee79c655c1d545706/chrome/browser/flag-metadata.json

[victorvianna via monorail]

Comment #7 on issue 1493956 by victor...@google.com: Stop assuming @chromium.org for flag-metadata.json owners
https://bugs.chromium.org/p/chromium/issues/detail?id=1493956#c7

I understood why #c5 was needed but I was puzzled by #c6. That invalid entry was added in r1212522, hours after the support was dropped in r1212140. Why did CQ not stop the CL?

Answer: It's the same root cause as before, an old CQ run was reused.
CQ+2 in r1212522 : https://luci-change-verifier.appspot.com/ui/run/chromium/8925205521854-1-bce47e6b13f0560d
See how most jobs are in "Tryjob reused". Only the presubmits ran.
If you click "Previous CV run", you get: https://luci-change-verifier.appspot.com/ui/run/chromium/8925281598854-1-b89f3beab679225f
For that one, linux-rel ran AboutFlagsTest.* https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/1575114/overview
But "Revision" was r1211953, *before* the support was dropped.

[victorvianna via monorail]

Comment #8 on issue 1493956 by victor...@google.com: Stop assuming @chromium.org for flag-metadata.json owners
https://bugs.chromium.org/p/chromium/issues/detail?id=1493956#c8

CQ only reuses results for up to 24h. So after 3:39 PM GMT+1 today, there can be no such new instances.