Issue 722430 link

Starred by 1 user

Issue metadata

Status:	Fixed
Owner:	jeffcarp@chromium.org
Closed:	Jun 2017
Cc:	qyears...@chromium.org
Components:	Blink>Infra>Ecosystem
EstimatedDays:	----
NextAction:	----
OS:	----
Pri:	1
Type:	Bug

Blocking:
issue 707006

[WPT Export] Landed Gerrit CLs cause a new PR to be created

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

Issue description

When crrev.com/c/479676 was landed, instead of merging the PR that had been created while in-flight [1], the exporter created a new PR and merged it [2]

[1] https://github.com/w3c/web-platform-tests/pull/5898
[2] https://github.com/w3c/web-platform-tests/pull/5914

This is most likely due to a missing conditional when the exporter is enumerating exportable commits in Chromium. We need to check if a given exportable commit has a corresponding PR coming from Gerrit, and if so, attempt to merge it.

Project Member

Comment 1 by bugdroid1@chromium.org, May 16 2017

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

commit b52f9c883a550145a4bd11456bb8639f2382f0c0
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Tue May 16 00:43:32 2017

[WPT Export] Fix exporter not catching in-flight Gerrit PRs

Bug:  722430 
Change-Id: I1fb3b2cb5e89aba1af48f38dc5c1c906ff9885ed
Reviewed-on: https://chromium-review.googlesource.com/506367
Commit-Queue: Jeff Carpenter <jeffcarp@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471958}
[modify] https://crrev.com/b52f9c883a550145a4bd11456bb8639f2382f0c0/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit.py
[modify] https://crrev.com/b52f9c883a550145a4bd11456bb8639f2382f0c0/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter.py
[modify] https://crrev.com/b52f9c883a550145a4bd11456bb8639f2382f0c0/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_exporter_unittest.py

Comment 2 by jeffcarp@chromium.org, May 16 2017

I'm going to keep this open until the next provisional PR lands so we can verify.

Comment 3 by jeffcarp@chromium.org, May 16 2017

Labels: -Pri-1 Pri-3

Comment 4 by jeffcarp@chromium.org, May 24 2017

Labels: -Pri-3 Pri-1

This is still an issue:

- https://github.com/w3c/web-platform-tests/pull/6043
- https://github.com/w3c/web-platform-tests/pull/6032

Comment 5 by jeffcarp@chromium.org, May 31 2017

Status: Started (was: Assigned)

Comment 6 by jeffcarp@chromium.org, Jun 1 2017

Status: Fixed (was: Started)

YES it's working now. The exporter correctly identified and merged this provisional PR this morning after the CL landed:
https://github.com/w3c/web-platform-tests/pull/6112

Comment 7 by qyears...@chromium.org, Jun 1 2017

Excellent :-D

Comment 8 by foolip@chromium.org, Jul 3 2017

Components: Blink>Infra>Ecosystem

Comment 9 by foolip@chromium.org, Jul 3 2017

Components: -Blink>Infra>Predictability

►

Issue 722430 link

Issue metadata

[WPT Export] Landed Gerrit CLs cause a new PR to be created

Issue description

Comment 1 by bugdroid1@chromium.org, May 16 2017

Comment 1 Deleted

Comment 2 by jeffcarp@chromium.org, May 16 2017

Comment 2 Deleted

Comment 3 by jeffcarp@chromium.org, May 16 2017

Comment 3 Deleted

Comment 4 by jeffcarp@chromium.org, May 24 2017

Comment 4 Deleted

Comment 5 by jeffcarp@chromium.org, May 31 2017

Comment 5 Deleted

Comment 6 by jeffcarp@chromium.org, Jun 1 2017

Comment 6 Deleted

Comment 7 by qyears...@chromium.org, Jun 1 2017

Comment 7 Deleted

Comment 8 by foolip@chromium.org, Jul 3 2017

Comment 8 Deleted

Comment 9 by foolip@chromium.org, Jul 3 2017

Comment 9 Deleted

Sign in to add a comment