From baa13eb8e32f4b4111e2ad1787db81221a80cc4f Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Tue, 19 Aug 2025 14:59:38 -0700 Subject: [PATCH 1/4] github: add PR template --- .github/pull_request_template.md | 54 ++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000000..9ed679978dda --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,54 @@ +Thank you for your PR. Please follow the steps in this template to ensure a +swift review. + +1. Read and follow the guidelines for contributing here: + https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md + + Note: if you are submitting a PR that does not address an open issue with an + agreed resolution, it is much more likely your PR will be rejected. + +2. 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. + +3. 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. + +4. 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. + +5. 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`. + +6. Self-review your code changes before sending your PR. + +7. Delete all of the above before sending your PR. \ No newline at end of file From 89d67aaf2a9adf82a87294307a7e4582b0ee51fe Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Tue, 19 Aug 2025 15:21:31 -0700 Subject: [PATCH 2/4] add newline at EOF and section on tests/checkers --- .github/pull_request_template.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 9ed679978dda..a938e4fcdf36 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -51,4 +51,16 @@ swift review. 6. Self-review your code changes before sending your PR. -7. Delete all of the above before sending your PR. \ No newline at end of file +7. All tests must be passing or you PR cannot be merged. There are two github + checkers 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. + +8. Delete all of the above before sending your PR. From 416689a68649eb6af62c5f7a9f16d216cc25d634 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Tue, 19 Aug 2025 15:35:22 -0700 Subject: [PATCH 3/4] typo --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index a938e4fcdf36..3da389dd38fd 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -29,7 +29,7 @@ swift review. ``` RELEASE NOTES: - * : + * : ``` This need not match the PR title. From a46aa627dde1b977929df69eadac1d307101cd6f Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Tue, 19 Aug 2025 16:47:54 -0700 Subject: [PATCH 4/4] move most text to CONTRIBUTING.md and ref that instead --- .github/pull_request_template.md | 70 ++-------------------- CONTRIBUTING.md | 99 +++++++++++++++++++++++++------- 2 files changed, 82 insertions(+), 87 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3da389dd38fd..424c6a6ee551 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,66 +1,4 @@ -Thank you for your PR. Please follow the steps in this template to ensure a -swift review. - -1. Read and follow the guidelines for contributing here: - https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md - - Note: if you are submitting a PR that does not address an open issue with an - agreed resolution, it is much more likely your PR will be rejected. - -2. 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. - -3. 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. - -4. 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. - -5. 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`. - -6. Self-review your code changes before sending your PR. - -7. All tests must be passing or you PR cannot be merged. There are two github - checkers 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. - -8. Delete all of the above before sending your PR. +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 1de0ce66691d..c064968bd5d6 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.