diff --git a/docs/maintainers/Images/merge-commit-confirm.png b/docs/maintainers/Images/merge-commit-confirm.png new file mode 100644 index 0000000000..23c26f939c Binary files /dev/null and b/docs/maintainers/Images/merge-commit-confirm.png differ diff --git a/docs/maintainers/Images/merge-commit.png b/docs/maintainers/Images/merge-commit.png new file mode 100644 index 0000000000..cf63266596 Binary files /dev/null and b/docs/maintainers/Images/merge-commit.png differ diff --git a/docs/maintainers/Images/squash-confirm.png b/docs/maintainers/Images/squash-confirm.png new file mode 100644 index 0000000000..2fe543b927 Binary files /dev/null and b/docs/maintainers/Images/squash-confirm.png differ diff --git a/docs/maintainers/Images/squash-merge.png b/docs/maintainers/Images/squash-merge.png new file mode 100644 index 0000000000..95c8635d8f Binary files /dev/null and b/docs/maintainers/Images/squash-merge.png differ diff --git a/docs/maintainers/README.md b/docs/maintainers/README.md index 82dee5ec45..1db04a76fd 100644 --- a/docs/maintainers/README.md +++ b/docs/maintainers/README.md @@ -20,14 +20,10 @@ They have [write access](https://help.github.com/articles/repository-permission- ## Current Repository Maintainers -* Sergei Vorobev ([vors](https://github.com/vors)) -* Jason Shirk ([lzybkr](https://github.com/lzybkr)) -* Dongbo Wang ([daxian-dbw](https://github.com/daxian-dbw)) -* Travis Plunk ([TravisEz13](https://github.com/TravisEz13)) -* Mike Richmond ([mirichmo](https://github.com/mirichmo)) -* Andy Schwartzmeyer ([andschwa](https://github.com/andschwa)) -* Aditya Patwardhan ([adityapatwardhan](https://github.com/adityapatwardhan)) -* Ilya Sazonov ([iSazonov](https://github.com/iSazonov)) +- Dongbo Wang ([daxian-dbw](https://github.com/daxian-dbw)) +- Travis Plunk ([TravisEz13](https://github.com/TravisEz13)) +- Aditya Patwardhan ([adityapatwardhan](https://github.com/adityapatwardhan)) +- Ilya Sazonov ([iSazonov](https://github.com/iSazonov)) ## Repository Maintainer Responsibilities @@ -73,6 +69,10 @@ Please see [Issue Management][issue-management] Please see [Contributing][CONTRIBUTING] +## Maintainer Best Practices + +Please see [Best Practices][best-practice] + ## Becoming a Repository Maintainer Repository Maintainers currently consist mostly of Microsoft employees. @@ -89,3 +89,4 @@ the [Current Repository Maintainers](#Current-Repository-Maintainers) with justi [ci-system]: ../testing-guidelines/testing-guidelines.md#ci-system [issue-management]: issue-management.md [CONTRIBUTING]: ../../.github/CONTRIBUTING.md +[best-practice]: best-practice.md diff --git a/docs/maintainers/best-practice.md b/docs/maintainers/best-practice.md new file mode 100644 index 0000000000..c1251aa463 --- /dev/null +++ b/docs/maintainers/best-practice.md @@ -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. + + ![](./Images/squash-merge.png)    ![](./Images/squash-confirm.png) + +- 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`. + + ![](./Images/merge-commit.png)    ![](./Images/merge-commit-confirm.png) + +- 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.