New issue
Advanced search Search tips

Issue 756202 link

Starred by 3 users

Issue metadata

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



Sign in to add a comment

[WPT common] Support multiple changes in one PR

Project Member Reported by robertma@chromium.org, Aug 16 2017

Issue description

There are legitimate scenarios people (on call) would like to manually export multiple commits in one PR, e.g. in order to pass the Travis CI.

The exporter needs to know these commits have been exported and not to try exporting them again. The correspondence between changes and PRs are established via Change-Id -- the exporter looks for change IDs in description of PRs.

The only problem here is that currently the exporter assumes one PR always contains one single change, with the change message in the PR description, so it only looks at the first Change-Id in the description. We can modify the lookup logic to look for all Change-Id in the description. In that way, when doing manual exports of multiple changes, we can simply put all change IDs (or concatenation of all change messages) in the PR description.

 
Cc: qyears...@chromium.org foolip@chromium.org
Labels: -Pri-3 Pri-2
Owner: robertma@chromium.org
Status: Assigned (was: Available)
Bumping to P2 in order to turn wpt-exporter green (reduce false alarms) since  issue 743149  has landed.
Cc: robertma@chromium.org jeffcarp@chromium.org
 Issue 756383  has been merged into this issue.
Labels: -Pri-2 Pri-1
Status: Started (was: Assigned)
Summary: [WPT common] Support multiple changes in one PR (was: [WPT Export] Support multiple changes in one PR)
After  issue 754613  is fixed. This issue now blocks automatic imports, because "Allow Date.now() rounding errors in timeOrigin test (https://chromium.googlesource.com/chromium/src/+/ed96173eaf)" is mistakenly considered exportable despite it already exported with another CL in one PR.

Example: https://luci-milo.appspot.com/buildbot/chromium.infra.cron/wpt-importer/583
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 28 2017

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

commit ef0902b9ad6a44078dd366aa783e87320fafe92d
Author: Robert Ma <robertma@chromium.org>
Date: Mon Aug 28 16:04:34 2017

Support mulitple CLs in one PR

If multiple CLs have to go into one PR, simply put their Change-Id footers
(or Cr-Commit-Position, if a CL does not have Change-Id) in the PR
description (make sure each takes up a whole line), and the PR is labelled
as "chromium-export".

Bug:  756202 
Change-Id: I7553d978edc80829dcad66590b09ada0c5a1af7b
Reviewed-on: https://chromium-review.googlesource.com/636315
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497768}
[modify] https://crrev.com/ef0902b9ad6a44078dd366aa783e87320fafe92d/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github.py
[modify] https://crrev.com/ef0902b9ad6a44078dd366aa783e87320fafe92d/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github_mock.py
[modify] https://crrev.com/ef0902b9ad6a44078dd366aa783e87320fafe92d/third_party/WebKit/Tools/Scripts/webkitpy/w3c/wpt_github_unittest.py

Status: Fixed (was: Started)
This is now fixed. The newest import is no longer blocked (as described in comment 4): https://build.chromium.org/p/chromium.infra.cron/builders/wpt-importer/builds/983

The exporter should no longer give false alarms as well -- red means something is actually wrong!

Sign in to add a comment