-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Agree on policy on reverting #9953
Comments
What I think personally:
As for what to bypass, Travis breakage obviously need to bypass Travis. For the other cases, I think it comes down to whether waiting up to 50 minutes makes a difference, leaning towards bypassing if someone requests it. CSSWG tooling, then. When it accidentally breaks, and CSSWG members asks for a revert, then I think we should revert. But I think we also need a way of reliably getting those issues resolved in a timely manner so that we can reland. I don't think we can come up with a constant number of days that we wait and then reland anyway, but I would hope we could coordinate and expect it to be resolved within a week if the change is something pretty superficial like how files are moved around, and not something like "let's break all reftests" (example!)... |
@boazsender @mariestaver, I imagine you don't disagree with reverting for the "actively trying to run frequently and more reliably" case? |
So I think when changes to infrastructure break the ability to run tests it may be appropriate to revert changes if doing so provides the fastest path to resolving the situation. There are all sorts of edge cases here like changes that fix previously broken A but break previosuly working B, where there's some decision required as to whether A or B is more important. I think one could formulate a more detailed policy here and specify tiers and so on, but I'm not sure how much value there is in that compared to rough consensus. However I think the case in hand is very different. What we had was a change to tests which conformed to the general rules for the project, but happened to break a legacy system which imposes its own WG-specific requirements and is not being actively improved to match the rest of the project. In that case I think reverting the patch is OK as long as the changes are relatively low value for the other participants, but such an action needs to be exceptional and there needs to be a clear plan to reland within a short timeframe. In general it is important to the success of the project that people are able to land changes to tests either in wpt directly or via browser repositories without memorising a large set of project rules, and without fear that their changes will be rejected for non-obvious reasons. It is an fundamental and longstanding tenent of wpt that contribution must not be substantially more difficult than for a vendor-specific testsuite, which is why there has been so much effort on two-way sync and improving the developer workflow. |
@foolip Our policy is broadly to "revert when something breaks a run that previously worked". But as both you and @jgraham point out, there are a lot of potential edge-cases here! So far it's made the most sense to resolve those cases in conversation, and maybe with a group decision if it's a particularly tricky case (like "this breaks A but fixes B, which is more important", for example). We're also happy to listen if someone has an urgent need to reland a PR, but again, the details of doing this feels like it'll come from conversation rather than policy. However, I'm happy to help you create a more specific policy if you think that will potentially avoid confusion -- let me know if I can assist. |
@jgraham, I agree, I'd like to have zero lints that only apply to css/, no external build system and a https://wpt.fyi/ that fulfills the needs of CSS folks. But AFAICT nobody is really spending much time working towards that goal, so what do we do in the meantime? @plinss, do you have something like an importance-ordered list of things that this repo and https://wpt.fyi/ would have to support in order for the old build system to be abandoned? I know of some things, but not their real importance. Should I ask www-style? |
Ping @plinss on CSS build system requirements in #9953 (comment). (I'm reminded of this issue by #12482.) |
I've sent #12493 to document what I think are reasonable guidelines. Everyone in this issue, please review! |
#12493 has a bunch of approvals. To allow for more comments/objections, I'll leave the PR up until Friday afternoon (~ 15:00 UTC) before merging. |
…anges, a=testonly Automatic update from web-platform-testsDocument rough consensus on reverting changes (#12493) Fixes web-platform-tests/wpt#9953. -- wpt-commits: 7abda4e7227b65fba2452980d106ed5f11d51b2b wpt-pr: 12493
…anges, a=testonly Automatic update from web-platform-testsDocument rough consensus on reverting changes (#12493) Fixes web-platform-tests/wpt#9953. -- wpt-commits: 7abda4e7227b65fba2452980d106ed5f11d51b2b wpt-pr: 12493
…anges, a=testonly Automatic update from web-platform-testsDocument rough consensus on reverting changes (#12493) Fixes web-platform-tests/wpt#9953. -- wpt-commits: 7abda4e7227b65fba2452980d106ed5f11d51b2b wpt-pr: 12493 UltraBlame original commit: 4b4863b6778762764d7d8f9b4cb37f946019d800
…anges, a=testonly Automatic update from web-platform-testsDocument rough consensus on reverting changes (#12493) Fixes web-platform-tests/wpt#9953. -- wpt-commits: 7abda4e7227b65fba2452980d106ed5f11d51b2b wpt-pr: 12493 UltraBlame original commit: 4b4863b6778762764d7d8f9b4cb37f946019d800
…anges, a=testonly Automatic update from web-platform-testsDocument rough consensus on reverting changes (#12493) Fixes web-platform-tests/wpt#9953. -- wpt-commits: 7abda4e7227b65fba2452980d106ed5f11d51b2b wpt-pr: 12493 UltraBlame original commit: 4b4863b6778762764d7d8f9b4cb37f946019d800
#9718 was a bit of non-fun, and I believe we need to agree on and document when we revert things and how fast it should be. This was recently also an issue with #8614, where the time between breakage and fix was longish, although nobody then considered reverting AFAICT.
So:
The text was updated successfully, but these errors were encountered: