[WPT Export] Set PR status to prevent merging on preliminary in-flight PRs |
|||||||||||
Issue descriptionToday the new export bot leaves (https://github.com/w3c/web-platform-tests/pull/5928#issuecomment-301623211) a comment like this: "This PR corresponds to a currently in-flight CL: https://chromium-review.googlesource.com/c/506009/ Please don't merge it manually - once the upstream CL lands and if Travis is green, the exporter will auto-merge it." I think this might still be confusing to people who don't know about the export process. Let's discuss alternate wording here. How about: "This PR corresponds to a currently in-flight chromium CL and is being reviewed at https://chromium-review.googlesource.com/c/506009/ according to the policy described at https://github.com/w3c/web-platform-tests#test-review. Comments in the PR here unfortunately may not be noticed by the developer working on the change, so please leave any comments upstream instead. Also this PR will be automatically merged when the upstream CL lands, so please do not merge it manually." Also perhaps we should add some text to the commit description to clarify that the description reflects not just the test change but the upstream chromium change as well? Eg. even just "Chromium export: " or "Export: " at the beginning of the first line? Or maybe a 2nd line of the form: "This commit represents the web-platform-test portion of a chromium commit with the following description"?
,
May 18 2017
The first comment you linked to was just me commenting manually 😅 I agree prefacing the title would help. @James, that sounds like a more robust solution!
,
May 19 2017
,
Jun 1 2017
,
Jun 5 2017
,
Jul 3 2017
,
Jul 3 2017
,
Jul 6 2017
This is done now with "do not merge yet", please reopen if something more is planned.
,
Jul 6 2017
jgraham says 'foolip: "Do Not Merge Yet" is not enforced. IMO the bot should set a GH status preventing merge' on #testing
,
Jul 6 2017
And comment 1 has the details for how this would work.
,
Jul 6 2017
I don't think I've seen another instance of people prematurely merging an export PR since the original issue (maybe now the process is more widely understood).
,
Aug 3 2017
,
Aug 6
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 6
robertma@chromium.org - is this something you're considering implementing?
,
Aug 6
I think this has already been fixed by the "do not merge yet" tag.
,
Aug 7
Sorry, I didn't read the issue carefully enough. It's specifically referring to the PR "status" (checks) on GitHub instead of relying on some conventional tags. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by ja...@hoppipolla.co.uk
, May 18 2017I wonder if instead of the text asking people not to merge, you could just set the status on GitHub to prevent (non-admin) merges. Then the text could focus on being informative rather than instructive. Seems like POSTing something to /repos/w3c/web-platform-tests/statuses/<sha> with a body like {"state"; "failure", "target_url": <link to review>, "description": "Pull request merged into Chromium", "context": "chrome"} would do the trick.