Adding a PR checklist to add guidance/reminders for authors to check for recurring PR review feedback. (#735)

It is generally useful for PR authors to go through their own PR first to make sure that commonly occurring feedback and issues have already been addressed. This checklist should help remind contributors what to look out for and is a living doc that we can modify and add to.

I ask these questions to myself before submitting a PR, when looking through my changes in the "pre-PR-submit-view".

Where possible leverage linter tools and CI gates to help address some of these. For example, for spell-check, here are some solutions to make things easier within the developer inner-loop:
- VS Code extension: [Code Spell Checker](https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker)
- VS Code extension: [Markdown linter](https://github.com/DavidAnson/vscode-markdownlint)
- Browser extension: [Site Spell](https://www.site-spell.com/)
This commit is contained in:
Ahson Khan 2020-10-13 18:00:06 -07:00 committed by GitHub
parent a38de03e7b
commit 139af60cc0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 10 deletions

View File

@ -0,0 +1,20 @@
# Pull Request Checklist
Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:
See the detailed list in the [contributing guide](https://github.com/Azure/azure-sdk-for-cpp/blob/master/CONTRIBUTING.md#pull-requests).
- [ ] [C++ Guidelines](https://azure.github.io/azure-sdk/cpp_introduction.html)
- [ ] Doxygen docs
- [ ] Unit tests
- [ ] No unwanted commits/changes
- [ ] Descriptive title/description
- [ ] PR is single purpose
- [ ] Related issue listed
- [ ] Comments in source
- [ ] No typos
- [ ] Update changelog
- [ ] Not work-in-progress
- [ ] External references or docs updated
- [ ] Self review of PR done
- [ ] Any breaking changes?

View File

@ -14,16 +14,24 @@ Thank you for your interest in contributing to Azure SDK for C++.
Pull Requests
-------------
* **DO** submit all code changes via pull requests (PRs) rather than through a direct commit. PRs will be reviewed and potentially merged by the repo maintainers after a peer review that includes at least one maintainer.
* **DO NOT** submit "work in progress" PRs. A PR should only be submitted when it is considered ready for review and subsequent merging by the contributor.
* **DO** give PRs short-but-descriptive names (e.g. "Improve code coverage for Azure.Core by 10%", not "Fix #1234")
* **DO** refer to any relevant issues, and include [keywords](https://help.github.com/articles/closing-issues-via-commit-messages/) that automatically close issues when the PR is merged.
* **DO** tag any users that should know about and/or review the change.
* **DO** ensure each commit successfully builds. The entire PR must pass all tests in the Continuous Integration (CI) system before it'll be merged.
* **DO** address PR feedback in an additional commit(s) rather than amending the existing commits, and only rebase/squash them when necessary. This makes it easier for reviewers to track changes.
* **DO** assume that ["Squash and Merge"](https://github.com/blog/2141-squash-your-commits) will be used to merge your commit unless you request otherwise in the PR.
* **DO NOT** fix merge conflicts using a merge commit. Prefer `git rebase`.
* **DO NOT** mix independent, unrelated changes in one PR. Separate real product/test code changes from larger code formatting/dead code removal changes. Separate unrelated fixes into separate PRs, especially if they are in different assemblies.
- **DO** follow the API design and implementation [C++ Guidelines](https://azure.github.io/azure-sdk/cpp_introduction.html).
- When submitting large changes or features, **DO** have an issue or spec doc that describes the design, usage, and motivating scenario.
- **DO** submit all code changes via pull requests (PRs) rather than through a direct commit. PRs will be reviewed and potentially merged by the repo maintainers after a peer review that includes at least one maintainer.
- **DO** review your own PR to make sure there aren't any unintended changes or commits before submitting it.
- **DO NOT** submit "work in progress" PRs. A PR should only be submitted when it is considered ready for review and subsequent merging by the contributor.
- If the change is work-in-progress or an experiment, **DO** start if off as a temporary draft PR.
- **DO** give PRs short-but-descriptive names (e.g. "Improve code coverage for Azure.Core by 10%", not "Fix #1234") and add a description which explains why the change is being made.
- **DO** refer to any relevant issues, and include [keywords](https://help.github.com/articles/closing-issues-via-commit-messages/) that automatically close issues when the PR is merged.
- **DO** tag any users that should know about and/or review the change.
- **DO** ensure each commit successfully builds. The entire PR must pass all tests in the Continuous Integration (CI) system before it'll be merged.
- **DO** address PR feedback in an additional commit(s) rather than amending the existing commits, and only rebase/squash them when necessary. This makes it easier for reviewers to track changes.
- **DO** assume that ["Squash and Merge"](https://github.com/blog/2141-squash-your-commits) will be used to merge your commit unless you request otherwise in the PR.
- **DO NOT** mix independent, unrelated changes in one PR. Separate real product/test code changes from larger code formatting/dead code removal changes. Separate unrelated fixes into separate PRs, especially if they are in different modules or files that otherwise wouldn't be changed.
- **DO** comment your code focusing on "why", where necessary. Otherwise, aim to keep it self-documenting with appropriate names and style.
- **DO** add [doxygen style API comments](https://azure.github.io/azure-sdk/cpp_documentation.html#docstrings) when adding new APIs or modifying header files.
- **DO** make sure there are no typos or spelling errors, especially in user-facing documentation.
- **DO** verify if your changes have impact elsewhere. For instance, do you need to update other docs or exiting markdown files that might be impacted?
- **DO** add relevant unit tests to ensure CI will catch future regressions.
Merging Pull Requests (for project contributors with write access)
----------------------------------------------------------