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

Issue 727914 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 707006



Sign in to add a comment

[WPT Export] Only create PRs for Gerrit CLs with reviewers

Project Member Reported by jeffcarp@chromium.org, May 30 2017

Issue description

People don't expect the exporter to come along and make a PR before it's sent for reviews, as was the case with this CL:
https://chromium-review.googlesource.com/c/517774/
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 9 2017

Created a CL to verify this:
https://chromium-review.googlesource.com/c/530191/
The CL didn't work >:(
https://github.com/w3c/web-platform-tests/pull/6208
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability
Similar to  issue 738891 , should perhaps be fixed as one?
The Gerrit API's ChangeInfo object returns a work_in_progress field, which could be useful in fixing this:
https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-info
Cc: b...@bocoup.com foolip@chromium.org jeffcarp@chromium.org
 Issue 738891  has been merged into this issue.
Cc: mkwst@chromium.org
So there are two boolean flags on ChangeInfo we could use:

"work_in_progress"
"has_review_started"

I'm not sure in what cases these can differ, but I'm going to go ahead and make a CL that relies on "has_review_started," since that better approximates the intent behind this issue.
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 6 2017

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

commit 62e0417223f6821f7ddb849abb5e3b2f864ec0d9
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Thu Jul 06 19:51:56 2017

[WPT Export] Prevent PRs from WIP CLs from being created

Originally we were relying on a CL's reviewer list to prevent creating
upstream PRs for CLs without reviewers, however in practice that hasn't
been working. I believe using the ChangeInfo property has_started_review
will be a more solid solution.

Bug:  727914 
Change-Id: Ibc2e06d46ba3a16bbceda3a271b675769b040c68
Reviewed-on: https://chromium-review.googlesource.com/559836
Commit-Queue: Jeff Carpenter <jeffcarp@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484716}
[modify] https://crrev.com/62e0417223f6821f7ddb849abb5e3b2f864ec0d9/third_party/WebKit/Tools/Scripts/webkitpy/w3c/gerrit.py
[modify] https://crrev.com/62e0417223f6821f7ddb849abb5e3b2f864ec0d9/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
[modify] https://crrev.com/62e0417223f6821f7ddb849abb5e3b2f864ec0d9/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py

Sign in to add a comment