|
1 | 1 | # How to contribute
|
2 | 2 |
|
3 |
| -We definitely welcome your patches and contributions to gRPC! Please read the gRPC |
4 |
| -organization's [governance rules](https://github.com/grpc/grpc-community/blob/master/governance.md) |
5 |
| -and [contribution guidelines](https://github.com/grpc/grpc-community/blob/master/CONTRIBUTING.md) before proceeding. |
| 3 | +We welcome your patches and contributions to gRPC! Please read the gRPC |
| 4 | +organization's [governance |
| 5 | +rules](https://github.com/grpc/grpc-community/blob/master/governance.md) before |
| 6 | +proceeding. |
6 | 7 |
|
7 | 8 | If you are new to GitHub, please start by reading [Pull Request howto](https://help.github.com/articles/about-pull-requests/)
|
8 | 9 |
|
9 | 10 | ## Legal requirements
|
10 | 11 |
|
11 | 12 | In order to protect both you and ourselves, you will need to sign the
|
12 |
| -[Contributor License Agreement](https://identity.linuxfoundation.org/projects/cncf). |
| 13 | +[Contributor License |
| 14 | +Agreement](https://identity.linuxfoundation.org/projects/cncf). When you create |
| 15 | +your first PR, a link will be added as a comment that contains the steps needed |
| 16 | +to complete this process. |
13 | 17 |
|
14 |
| -## Guidelines for Pull Requests |
15 |
| -How to get your contributions merged smoothly and quickly. |
| 18 | +## Getting Started |
16 | 19 |
|
17 |
| -- Create **small PRs** that are narrowly focused on **addressing a single |
18 |
| - concern**. We often times receive PRs that are trying to fix several things at |
19 |
| - a time, but only one fix is considered acceptable, nothing gets merged and |
20 |
| - both author's & review's time is wasted. Create more PRs to address different |
21 |
| - concerns and everyone will be happy. |
| 20 | +A great way to start is by searching through our open issues. [Unassigned issues |
| 21 | +labeled as "help |
| 22 | +wanted"](https://github.com/grpc/grpc-go/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20label%3A%22Status%3A%20Help%20Wanted%22%20no%3Aassignee) |
| 23 | +are especially nice for first-time contributors, as they should be well-defined |
| 24 | +problems that already have agreed-upon solutions. |
22 | 25 |
|
23 |
| -- If you are searching for features to work on, issues labeled [Status: Help |
24 |
| - Wanted](https://github.com/grpc/grpc-go/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22Status%3A+Help+Wanted%22) |
25 |
| - is a great place to start. These issues are well-documented and usually can be |
26 |
| - resolved with a single pull request. |
| 26 | +## Code Style |
27 | 27 |
|
28 |
| -- If you are adding a new file, make sure it has the copyright message template |
29 |
| - at the top as a comment. You can copy over the message from an existing file |
30 |
| - and update the year. |
| 28 | +We follow [Google's published Go style |
| 29 | +guide](https://google.github.io/styleguide/go/). Note that there are three |
| 30 | +primary documents that make up this style guide; please follow them as closely |
| 31 | +as possible. If a reviewer recommends something that contradicts those |
| 32 | +guidelines, there may be valid reasons to do so, but it should be rare. |
31 | 33 |
|
32 |
| -- The grpc package should only depend on standard Go packages and a small number |
33 |
| - of exceptions. If your contribution introduces new dependencies which are NOT |
34 |
| - in the [list](https://godoc.org/google.golang.org/grpc?imports), you need a |
35 |
| - discussion with gRPC-Go authors and consultants. |
| 34 | +## Guidelines for Pull Requests |
36 | 35 |
|
37 |
| -- For speculative changes, consider opening an issue and discussing it first. If |
38 |
| - you are suggesting a behavioral or API change, consider starting with a [gRFC |
39 |
| - proposal](https://github.com/grpc/proposal). |
| 36 | +How to get your contributions merged smoothly and quickly: |
| 37 | + |
| 38 | +- Create **small PRs** that are narrowly focused on **addressing a single |
| 39 | + concern**. We often receive PRs that attempt to fix several things at the same |
| 40 | + time, and if one part of the PR has a problem, that will hold up the entire |
| 41 | + PR. |
| 42 | + |
| 43 | +- For **speculative changes**, consider opening an issue and discussing it |
| 44 | + first. If you are suggesting a behavioral or API change, consider starting |
| 45 | + with a [gRFC proposal](https://github.com/grpc/proposal). Many new features |
| 46 | + that are not bug fixes will require cross-language agreement. |
| 47 | + |
| 48 | +- If you want to fix **formatting or style**, consider whether your changes are |
| 49 | + an obvious improvement or might be considered a personal preference. If a |
| 50 | + style change is based on preference, it likely will not be accepted. If it |
| 51 | + corrects widely agreed-upon anti-patterns, then please do create a PR and |
| 52 | + explain the benefits of the change. |
| 53 | + |
| 54 | +- For correcting **misspellings**, please be aware that we use some terms that |
| 55 | + are sometimes flagged by spell checkers. As an example, "if an only if" is |
| 56 | + often written as "iff". Please do not make spelling correction changes unless |
| 57 | + you are certain they are misspellings. |
40 | 58 |
|
41 | 59 | - Provide a good **PR description** as a record of **what** change is being made
|
42 | 60 | and **why** it was made. Link to a GitHub issue if it exists.
|
43 | 61 |
|
44 |
| -- If you want to fix formatting or style, consider whether your changes are an |
45 |
| - obvious improvement or might be considered a personal preference. If a style |
46 |
| - change is based on preference, it likely will not be accepted. If it corrects |
47 |
| - widely agreed-upon anti-patterns, then please do create a PR and explain the |
48 |
| - benefits of the change. |
| 62 | +- Maintain a **clean commit history** and use **meaningful commit messages**. |
| 63 | + PRs with messy commit histories are difficult to review and won't be merged. |
| 64 | + Before sending your PR, ensure your changes are based on top of the latest |
| 65 | + `upstream/master` commits, and avoid rebasing in the middle of a code review. |
| 66 | + You should **never use `git push -f`** unless absolutely necessary during a |
| 67 | + review, as it can interfere with GitHub's tracking of comments. |
49 | 68 |
|
50 |
| -- Unless your PR is trivial, you should expect there will be reviewer comments |
51 |
| - that you'll need to address before merging. We'll mark it as `Status: Requires |
52 |
| - Reporter Clarification` if we expect you to respond to these comments in a |
53 |
| - timely manner. If the PR remains inactive for 6 days, it will be marked as |
54 |
| - `stale` and automatically close 7 days after that if we don't hear back from |
55 |
| - you. |
| 69 | +- **All tests need to be passing** before your change can be merged. We |
| 70 | + recommend you run tests locally before creating your PR to catch breakages |
| 71 | + early on: |
56 | 72 |
|
57 |
| -- Maintain **clean commit history** and use **meaningful commit messages**. PRs |
58 |
| - with messy commit history are difficult to review and won't be merged. Use |
59 |
| - `rebase -i upstream/master` to curate your commit history and/or to bring in |
60 |
| - latest changes from master (but avoid rebasing in the middle of a code |
61 |
| - review). |
| 73 | + - `./scripts/vet.sh` to catch vet errors. |
| 74 | + - `go test -cpu 1,4 -timeout 7m ./...` to run the tests. |
| 75 | + - `go test -race -cpu 1,4 -timeout 7m ./...` to run tests in race mode. |
62 | 76 |
|
63 |
| -- Keep your PR up to date with upstream/master (if there are merge conflicts, we |
64 |
| - can't really merge your change). |
| 77 | + Note that we have a multi-module repo, so `go test` commands may need to be |
| 78 | + run from the root of each module in order to cause all tests to run. |
65 | 79 |
|
66 |
| -- **All tests need to be passing** before your change can be merged. We |
67 |
| - recommend you **run tests locally** before creating your PR to catch breakages |
68 |
| - early on. |
69 |
| - - `./scripts/vet.sh` to catch vet errors |
70 |
| - - `go test -cpu 1,4 -timeout 7m ./...` to run the tests |
71 |
| - - `go test -race -cpu 1,4 -timeout 7m ./...` to run tests in race mode |
| 80 | + *Alternatively*, you may find it easier to push your changes to your fork on |
| 81 | + GitHub, which will trigger a GitHub Actions run that you can use to verify |
| 82 | + everything is passing. |
| 83 | + |
| 84 | +- If you are adding a new file, make sure it has the **copyright message** |
| 85 | + template at the top as a comment. You can copy the message from an existing |
| 86 | + file and update the year. |
| 87 | + |
| 88 | +- The grpc package should only depend on standard Go packages and a small number |
| 89 | + of exceptions. **If your contribution introduces new dependencies**, you will |
| 90 | + need a discussion with gRPC-Go maintainers. A github action check will run on |
| 91 | + every PR, and will flag any transitive dependency changes from any public |
| 92 | + package. |
| 93 | + |
| 94 | +- Unless your PR is trivial, you should **expect reviewer comments** that you |
| 95 | + will need to address before merging. We'll label the PR as `Status: Requires |
| 96 | + Reporter Clarification` if we expect you to respond to these comments in a |
| 97 | + timely manner. If the PR remains inactive for 6 days, it will be marked as |
| 98 | + `stale`, and we will automatically close it after 7 days if we don't hear back |
| 99 | + from you. Please feel free to ping issues or bugs if you do not get a response |
| 100 | + within a week. |
72 | 101 |
|
73 |
| -- Exceptions to the rules can be made if there's a compelling reason for doing so. |
| 102 | +- Exceptions to the rules can be made if there's a compelling reason to do so. |
0 commit comments