diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..424c6a6ee --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,4 @@ +Thank you for your PR. Please read and follow +https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md, especially the +"Guidelines for Pull Requests" section, and then delete this text before +entering your PR description. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1de0ce666..c064968bd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,17 +33,21 @@ guidelines, there may be valid reasons to do so, but it should be rare. ## Guidelines for Pull Requests -How to get your contributions merged smoothly and quickly: +Please read the following carefully to ensure your contributions can be merged +smoothly and quickly. + +### PR Contents - Create **small PRs** that are narrowly focused on **addressing a single concern**. We often receive PRs that attempt to fix several things at the same time, and if one part of the PR has a problem, that will hold up the entire PR. -- For **speculative changes**, consider opening an issue and discussing it - first. If you are suggesting a behavioral or API change, consider starting - with a [gRFC proposal](https://github.com/grpc/proposal). Many new features - that are not bug fixes will require cross-language agreement. +- If your change does not address an **open issue** with an **agreed + resolution**, consider opening an issue and discussing it first. If you are + suggesting a behavioral or API change, consider starting with a [gRFC + proposal](https://github.com/grpc/proposal). Many new features that are not + bug fixes will require cross-language agreement. - If you want to fix **formatting or style**, consider whether your changes are an obvious improvement or might be considered a personal preference. If a @@ -56,16 +60,6 @@ How to get your contributions merged smoothly and quickly: often written as "iff". Please do not make spelling correction changes unless you are certain they are misspellings. -- Provide a good **PR description** as a record of **what** change is being made - and **why** it was made. Link to a GitHub issue if it exists. - -- Maintain a **clean commit history** and use **meaningful commit messages**. - PRs with messy commit histories are difficult to review and won't be merged. - Before sending your PR, ensure your changes are based on top of the latest - `upstream/master` commits, and avoid rebasing in the middle of a code review. - You should **never use `git push -f`** unless absolutely necessary during a - review, as it can interfere with GitHub's tracking of comments. - - **All tests need to be passing** before your change can be merged. We recommend you run tests locally before creating your PR to catch breakages early on: @@ -81,15 +75,80 @@ How to get your contributions merged smoothly and quickly: GitHub, which will trigger a GitHub Actions run that you can use to verify everything is passing. -- If you are adding a new file, make sure it has the **copyright message** +- Note that there are two github actions checks that need not be green: + + 1. We test the freshness of the generated proto code we maintain via the + `vet-proto` check. If the source proto files are updated, but our repo is + not updated, an optional checker will fail. This will be fixed by our team + in a separate PR and will not prevent the merge of your PR. + + 2. We run a checker that will fail if there is any change in dependencies of + an exported package via the `dependencies` check. If new dependencies are + added that are not appropriate, we may not accept your PR (see below). + +- If you are adding a **new file**, make sure it has the **copyright message** template at the top as a comment. You can copy the message from an existing file and update the year. - The grpc package should only depend on standard Go packages and a small number of exceptions. **If your contribution introduces new dependencies**, you will - need a discussion with gRPC-Go maintainers. A GitHub action check will run on - every PR, and will flag any transitive dependency changes from any public - package. + need a discussion with gRPC-Go maintainers. + +### PR Descriptions + +- **PR titles** should start with the name of the component being addressed, or + the type of change. Examples: transport, client, server, round_robin, xds, + cleanup, deps. + +- Read and follow the **guidelines for PR titles and descriptions** here: + https://google.github.io/eng-practices/review/developer/cl-descriptions.html + + *particularly* the sections "First Line" and "Body is Informative". + + Note: your PR description will be used as the git commit message in a + squash-and-merge if your PR is approved. We may make changes to this as + necessary. + +- **Does this PR relate to an open issue?** On the first line, please use the + tag `Fixes #` to ensure the issue is closed when the PR is merged. Or + use `Updates #` if the PR is related to an open issue, but does not fix + it. Consider filing an issue if one does not already exist. + +- PR descriptions *must* conclude with **release notes** as follows: + + ``` + RELEASE NOTES: + * : + ``` + + This need not match the PR title. + + The summary must: + + * be something that gRPC users will understand. + + * clearly explain the feature being added, the issue being fixed, or the + behavior being changed, etc. If fixing a bug, be clear about how the bug + can be triggered by an end-user. + + * begin with a capital letter and use complete sentences. + + * be as short as possible to describe the change being made. + + If a PR is *not* end-user visible -- e.g. a cleanup, testing change, or + github-related, use `RELEASE NOTES: n/a`. + +### PR Process + +- Please **self-review** your code changes before sending your PR. This will + prevent simple, obvious errors from causing delays. + +- Maintain a **clean commit history** and use **meaningful commit messages**. + PRs with messy commit histories are difficult to review and won't be merged. + Before sending your PR, ensure your changes are based on top of the latest + `upstream/master` commits, and avoid rebasing in the middle of a code review. + You should **never use `git push -f`** unless absolutely necessary during a + review, as it can interfere with GitHub's tracking of comments. - Unless your PR is trivial, you should **expect reviewer comments** that you will need to address before merging. We'll label the PR as `Status: Requires @@ -98,5 +157,3 @@ How to get your contributions merged smoothly and quickly: `stale`, and we will automatically close it after 7 days if we don't hear back from you. Please feel free to ping issues or bugs if you do not get a response within a week. - -- Exceptions to the rules can be made if there's a compelling reason to do so.