[WPT Export] Handle reverted changes |
||||||||||||
Issue description- [x] Create a tiny change in external/wpt - https://codereview.chromium.org/2651533003 - [x] Revert it - https://codereview.chromium.org/2657613002 - [ ] Run script and make necessary fixes
,
Jan 20 2017
,
Jan 20 2017
,
Jan 23 2017
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b156b16a00574a89d85ccb034b30cdfd9cc5676a commit b156b16a00574a89d85ccb034b30cdfd9cc5676a Author: jeffcarp <jeffcarp@chromium.org> Date: Tue Jan 24 04:54:57 2017 Create a change in external/wpt meant to be reverted This is to test reverted changes in the WPT Export process. BUG= 679951 Review-Url: https://codereview.chromium.org/2651533003 Cr-Commit-Position: refs/heads/master@{#445651} [modify] https://crrev.com/b156b16a00574a89d85ccb034b30cdfd9cc5676a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/forms/the-input-element/checkbox.html
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0702925bc3519350a1f578c40de225e45da52717 commit 0702925bc3519350a1f578c40de225e45da52717 Author: jeffcarp <jeffcarp@chromium.org> Date: Tue Jan 24 18:19:10 2017 Revert of Create a change in external/wpt meant to be reverted (patchset #1 id:1 of https://codereview.chromium.org/2651533003/ ) Reason for revert: This commit was intended to be reverted to test how WPT Export handles reverted changes. See the bug for more context. Original issue's description: > Create a change in external/wpt meant to be reverted > > This is to test reverted changes in the WPT Export process. > > BUG= 679951 > > Review-Url: https://codereview.chromium.org/2651533003 > Cr-Commit-Position: refs/heads/master@{#445651} > Committed: https://chromium.googlesource.com/chromium/src/+/b156b16a00574a89d85ccb034b30cdfd9cc5676a TBR=qyearsley@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 679951 Review-Url: https://codereview.chromium.org/2657613002 Cr-Commit-Position: refs/heads/master@{#445762} [modify] https://crrev.com/0702925bc3519350a1f578c40de225e45da52717/third_party/WebKit/LayoutTests/external/wpt/html/semantics/forms/the-input-element/checkbox.html
,
Jan 24 2017
Quinten, I'm looking for input on how to deal with reverted changes. I have two ideas so far, both of which involve doing two passes on the list of exportable commits: 1. Look to see if any commit in exportable_commits has a revert commit later on 2. Diff each commit against chromium ToT, if it doesn't make a patch then that commit has been reverted
,
Jan 24 2017
,
Jan 24 2017
I was thinking as I was working on this - what if we just didn't "handle" reverted changes? Both the original commit and the revert commit would go through the export process normally, resulting in a little bit more noise in the IRC channel. This would eliminate a good amount of complexity added by revert checking. And also, if a commit has already been exported, then we'd need to export the revert commit anyway, so this feature would only catch reverted commits within a ~20-30 minute window. I think that the tradeoff leans toward not building this feature to ignore reverted commits in this way, but I'm open to feedback. What's more worrisome for me is how to deal with conflicts upstream. I'll create a tracking bug for that now.
,
Jan 24 2017
Yeah, I agree -- the simpler the better, and even if it were fairly common for people to commit and then revert right afterwards, it probably wouldn't be extremely troublesome or unexpected for both the original commit and then revert to go through the export pipeline. Maybe we'll put this as a P3, a potential feature to consider at some point in the future? Re #7: The way I was imagining this might be done is #1 -- if, when the export script is run, there is both a commit and its revert commit, that are both exportable and not exported, then they cancel each other out, and neither are exported.
,
Jan 25 2017
,
Jan 25 2017
,
Mar 30 2017
,
Mar 30 2017
,
May 9 2017
In the 198 export commits we've seen about 4 reverts go through so far: https://github.com/w3c/web-platform-tests/pulls?utf8=%E2%9C%93&q=is%3Apr%20label%3Achromium-export%20%22Revert%20of%22%20in%3Atitle%20 My opinion is that this is an optimization that we can do if people start complaining about revert commits being PR'd upstream, in which case we can re-open this bug.
,
Jul 3 2017
,
Jul 3 2017
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by jeffcarp@chromium.org
, Jan 18 2017