-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add maintainers best practice document and update maintainer list #7311
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # Maintainer Best Practices | ||
|
|
||
| ## PR Types | ||
|
|
||
| - Feature-work PR: A PR that implements an RFC, which usually involves relatively large set of changes. | ||
| - Regular PR: A bug fix or an enhancement change that are not backed by an RFC. | ||
|
|
||
| ## Review PRs | ||
|
|
||
| - Ask the author to reword the PR title based on guidelines in [Contributing](../../.github/CONTRIBUTING.md). | ||
| - Ask the author to apply `[Feature]` tag to trigger full test builds if it's necessary. | ||
| - Label the PR with `Breaking-Change`, `Documentation Needed` and `Area-XXX` as appropriate. | ||
| - When labelling a PR with `Review-Committee`, leave a detailed comment to summarize the issue you want the committee to look into. | ||
| It's recommended to include examples to explain/demonstrate behaviors. | ||
|
|
||
| ## Merge PRs | ||
|
|
||
| - Use `Squash and merge` by default to keep clean commit history in Master branch. | ||
|
|
||
|   | ||
|
|
||
| - Use `Create a merge commit` for feature-work PRs **only if** the commit history of the PR is reasonably clean. | ||
| After using this option, Github will make it your default option for merging a PR. | ||
| So, do remember to change the default back to `Squash and merge`. | ||
|
|
||
|   | ||
|
|
||
| - Avoid `Rebase and merge` unless you have a strong argument for using it. | ||
|
|
||
| - Before clicking `Confirm squash and merge` or `Confirm merge`, | ||
| make sure you run through the following steps: | ||
|
|
||
| 1. The commit title should be a short summary of the PR. | ||
|
|
||
| - When merging with the `Squash and merge` option, | ||
| the PR title will be used as the commit title by default. | ||
| **Reword the title as needed** to make sure it makes sense (can be used without change in `CHANGELOG.md`). | ||
|
|
||
| - When merging with the `Create a merge commit` option, | ||
| the default commit title would be `Merge pull request XXX from YYY`. | ||
| **Replace it with a short summary of the PR**, and add the PR number to the end, like `(#1234)`. | ||
|
|
||
| 1. The optional extended description is required for feature-work PRs, or regular PRs with breaking changes. | ||
| For other PRs, it's not required but good to have based on the judgement of the maintainer. | ||
|
|
||
| - If a PR introduces breaking changes, | ||
| make sure you put the tag `[breaking change]` at the first line of the extended description, | ||
| and start the description text from the second line. | ||
|
|
||
| 1. Use the present tense and imperative mood for both the commit title and description. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ironically, in this case PR Summary does not entirely correctly summaries the changes. :)
PR Summary += "... and updating maintainer list"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Title updated.