Skip to content
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

Meta: add more guidance for how to handle test PRs #1938

Merged
merged 1 commit into from
Oct 20, 2016

Conversation

foolip
Copy link
Member

@foolip foolip commented Oct 20, 2016

Prompted by web-platform-tests/wpt#4034.

Part of #1849.

@@ -6,7 +6,7 @@ This document contains info used by the team maintaining the standard. Mostly bo

Each change needs to result in a single commit on the master branch, with no merge commits. The green squash and merge button is OK to use, but be sure to tidy up the commit message per [guidelines for writing good commit messages](https://github.com/erlang/otp/wiki/Writing-good-commit-messages).

For normative changes, ask for a [web-platform-tests](https://github.com/w3c/web-platform-tests) PR if testing is practical and not overly burdensome. If a follow up issue is filed, add the `html` label.
For normative changes, ask for a [web-platform-tests](https://github.com/w3c/web-platform-tests) PR if testing is practical and not overly burdensome. Merge both PRs at the same time. If a follow up issue is filed, add the `html` label.
Copy link
Member

@mathiasbynens mathiasbynens Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/follow up issue/follow-up issue/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

@foolip
Copy link
Member Author

foolip commented Oct 20, 2016

This is just my hunch of where we'll naturally end up, in each case doing things in whichever order seems best.

Not sure if more can be done to clarify that two PRs belong together, I guess the main risk is someone not in the loop merging a WPT change?

@zcorpan
Copy link
Member

zcorpan commented Oct 20, 2016

Maybe say to add the "do not merge yet" label (wpt has it also)

@foolip
Copy link
Member Author

foolip commented Oct 20, 2016

Done.

@zcorpan zcorpan removed their assignment Oct 20, 2016
@annevk
Copy link
Member

annevk commented Oct 20, 2016

Instead of typically, make it an instruction? "Attempt to merge both PRs at the same time."

@annevk
Copy link
Member

annevk commented Oct 20, 2016

Never mind, missed TEAM changes. LGTM

@zcorpan zcorpan merged commit e2ffc75 into master Oct 20, 2016
@zcorpan zcorpan deleted the more-test-guidance branch October 20, 2016 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants