Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2684663003:
WPT importer: List directory owners in CL description. (Closed)
DescriptionWPT importer: List directory owners in CL description.
For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this:
Directory owners for changes in this CL:
jsbell@chromium.org
external/wpt/hr-time
dom-dev@chromium.org
external/wpt/html
BUG=687270
Review-Url: https://codereview.chromium.org/2684663003
Cr-Commit-Position: refs/heads/master@{#449555}
Committed: https://chromium.googlesource.com/chromium/src/+/e0a0c33d8b44020aede5aa1c52fac85ce14b7f6e
Patch Set 1 #Patch Set 2 : Update change to not change TBR line. #
Messages
Total messages: 25 (15 generated)
Description was changed from ========== WPT importer: List directory owners in CL description and TBR line. BUG=687270 ========== to ========== WPT importer: List directory owners in CL description and TBR line. For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this: Directory owners for changes in this CL: jsbell@chromium.org external/wpt/hr-time dom-dev@chromium.org external/wpt/html TBR=qyearsley@chromium.org,dom-dev@chromium.org,jsbell@chromium.org Does this seem like what we want? BUG=687270 ==========
qyearsley@chromium.org changed reviewers: + jeffcarp@chromium.org
The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Do directory owners have a way of opting out of this? Will owners of frequently modified directories receive too much signal vs. noise?
On 2017/02/07 at 23:54:27, jeffcarp wrote: > Do directory owners have a way of opting out of this? Will owners of frequently modified directories receive too much signal vs. noise? Oops I meant too much noise over signal.
On 2017/02/07 at 23:54:27, jeffcarp wrote: > Do directory owners have a way of opting out of this? Will owners of frequently modified directories receive too much [noise vs. signal]? Very good question -- I'm not sure. Maybe it's better to keep directory owners as CC rather than reviewer; but at some point we'll want to have someone else regularly inspect and review these jsbell@, foolip@, kojii@: as directory owners in W3CImportExpectations, what do you think about being listed as reviewer for every single upstream change in the directories you own? Meanwhile, in the short term, if I change this CL to list owners and directories in the CL description but keep TBR=qyearsley for now, does that sound OK?
On 2017/02/08 02:14:52, qyearsley wrote: > On 2017/02/07 at 23:54:27, jeffcarp wrote: > > Do directory owners have a way of opting out of this? Will owners of > frequently modified directories receive too much [noise vs. signal]? > > Very good question -- I'm not sure. Maybe it's better to keep directory owners > as CC rather than reviewer; but at some point we'll want to have someone else > regularly inspect and review these > > jsbell@, foolip@, kojii@: as directory owners in W3CImportExpectations, what do > you think about being listed as reviewer for every single upstream change in the > directories you own? Even if the volume of email wasn't too bad, I suspect that just getting emails won't be enough to keep the imported tests in good shape over a long period of time, as inevitably one misses an email or 10. Even trying to file bugs that would be triaged wouldn't suffice, as some bugs would be accidentally or incorrectly closed. Given that the importer is already changing expectations, how much work would it be to create a per-directory report that gets sent to owners for the first import every week or something? > Meanwhile, in the short term, if I change this CL to list owners and directories > in the CL description but keep TBR=qyearsley for now, does that sound OK? That sounds OK, but would you have a use for this information? I suspect that the long-term test suite health isn't the only reason one might want this, but it's late and my brain is turning off. Good night :)
On 2017/02/08 at 05:49:38, foolip wrote: > Even if the volume of email wasn't too bad, I suspect that just getting emails won't be enough to keep the imported tests in good shape over a long period of time, as inevitably one misses an email or 10. Even trying to file bugs that would be triaged wouldn't suffice, as some bugs would be accidentally or incorrectly closed. > > Given that the importer is already changing expectations, how much work would it be to create a per-directory report that gets sent to owners for the first import every week or something? If we wanted to do that, I think we'd want to store the per-directory changes somewhere on import... this seems like it may be more complicated than it is worth. In any case, the main point of this CL is to make it easier for CCd directory owners to quickly check what files they should look at. I've now updated this CL to just add a list of changed directories and their owners, but not change the list of reviewers at this time.
The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
qyearsley@chromium.org changed reviewers: + tkent@chromium.org
lgtm
The CQ bit was checked by qyearsley@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486699985513570,
"parent_rev": "c39bf24ed5228e0da8b7cc1700e1cac6f4997b08", "commit_rev":
"e0a0c33d8b44020aede5aa1c52fac85ce14b7f6e"}
Message was sent while issue was closed.
Description was changed from ========== WPT importer: List directory owners in CL description and TBR line. For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this: Directory owners for changes in this CL: jsbell@chromium.org external/wpt/hr-time dom-dev@chromium.org external/wpt/html TBR=qyearsley@chromium.org,dom-dev@chromium.org,jsbell@chromium.org Does this seem like what we want? BUG=687270 ========== to ========== WPT importer: List directory owners in CL description and TBR line. For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this: Directory owners for changes in this CL: jsbell@chromium.org external/wpt/hr-time dom-dev@chromium.org external/wpt/html TBR=qyearsley@chromium.org,dom-dev@chromium.org,jsbell@chromium.org Does this seem like what we want? BUG=687270 Review-Url: https://codereview.chromium.org/2684663003 Cr-Commit-Position: refs/heads/master@{#449555} Committed: https://chromium.googlesource.com/chromium/src/+/e0a0c33d8b44020aede5aa1c52fa... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/e0a0c33d8b44020aede5aa1c52fa...
Message was sent while issue was closed.
Description was changed from ========== WPT importer: List directory owners in CL description and TBR line. For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this: Directory owners for changes in this CL: jsbell@chromium.org external/wpt/hr-time dom-dev@chromium.org external/wpt/html TBR=qyearsley@chromium.org,dom-dev@chromium.org,jsbell@chromium.org Does this seem like what we want? BUG=687270 Review-Url: https://codereview.chromium.org/2684663003 Cr-Commit-Position: refs/heads/master@{#449555} Committed: https://chromium.googlesource.com/chromium/src/+/e0a0c33d8b44020aede5aa1c52fa... ========== to ========== WPT importer: List directory owners in CL description. For example, if there are updates in the hr-time and html directories, this will make it so that import CL descriptions would include something like this: Directory owners for changes in this CL: jsbell@chromium.org external/wpt/hr-time dom-dev@chromium.org external/wpt/html BUG=687270 Review-Url: https://codereview.chromium.org/2684663003 Cr-Commit-Position: refs/heads/master@{#449555} Committed: https://chromium.googlesource.com/chromium/src/+/e0a0c33d8b44020aede5aa1c52fa... ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
