Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 1 | # Contributing to Protocol Buffers |
| 2 | |
Elliotte Rusty Harold | ab993cf | 2021-10-13 16:36:21 +0000 | [diff] [blame] | 3 | We welcome some types of contributions to protocol buffers. This doc describes the |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 4 | process to contribute patches to protobuf and the general guidelines we |
| 5 | expect contributors to follow. |
| 6 | |
Elliotte Rusty Harold | ab993cf | 2021-10-13 16:36:21 +0000 | [diff] [blame] | 7 | ## What We Accept |
| 8 | |
| 9 | * Bug fixes with unit tests demonstrating the problem are very welcome. |
| 10 | We also appreciate bug reports, even when they don't come with a patch. |
| 11 | Bug fixes without tests are usually not accepted. |
| 12 | * New APIs and features with adequate test coverage and documentation |
| 13 | may be accepted if they do not compromise backwards |
| 14 | compatibility. However there's a fairly high bar of usefulness a new public |
| 15 | method must clear before it will be accepted. Features that are fine in |
| 16 | isolation are often rejected because they don't have enough impact to justify the |
| 17 | conceptual burden and ongoing maintenance cost. It's best to file an issue |
| 18 | and get agreement from maintainers on the value of a new feature before |
| 19 | working on a PR. |
| 20 | * Performance optimizations may be accepted if they have convincing benchmarks that demonstrate |
| 21 | an improvement and they do not significantly increase complexity. |
| 22 | * Changes to existing APIs are almost never accepted. Stability and |
| 23 | backwards compatibility are paramount. In the unlikely event a breaking change |
| 24 | is required, it must usually be implemented in google3 first. |
| 25 | * Changes to the wire and text formats are never accepted. Any breaking change |
| 26 | to these formats would have to be implemented as a completely new format. |
| 27 | We cannot begin generating protos that cannot be parsed by existing code. |
| 28 | |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 29 | ## Before You Start |
| 30 | |
| 31 | We accept patches in the form of github pull requests. If you are new to |
| 32 | github, please read [How to create github pull requests](https://help.github.com/articles/about-pull-requests/) |
| 33 | first. |
| 34 | |
| 35 | ### Contributor License Agreements |
| 36 | |
| 37 | Contributions to this project must be accompanied by a Contributor License |
| 38 | Agreement. You (or your employer) retain the copyright to your contribution, |
| 39 | this simply gives us permission to use and redistribute your contributions |
| 40 | as part of the project. |
| 41 | |
| 42 | * If you are an individual writing original source code and you're sure you |
| 43 | own the intellectual property, then you'll need to sign an [individual CLA](https://cla.developers.google.com/about/google-individual?csw=1). |
| 44 | * If you work for a company that wants to allow you to contribute your work, |
| 45 | then you'll need to sign a [corporate CLA](https://cla.developers.google.com/about/google-corporate?csw=1). |
| 46 | |
| 47 | ### Coding Style |
| 48 | |
| 49 | This project follows [Google’s Coding Style Guides](https://github.com/google/styleguide). |
| 50 | Before sending out your pull request, please familiarize yourself with the |
| 51 | corresponding style guides and make sure the proposed code change is style |
| 52 | conforming. |
| 53 | |
| 54 | ## Contributing Process |
| 55 | |
noahdietz | 5abf802 | 2022-04-12 10:25:08 -0700 | [diff] [blame] | 56 | Most pull requests should go to the main branch and the change will be |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 57 | included in the next major/minor version release (e.g., 3.6.0 release). If you |
| 58 | need to include a bug fix in a patch release (e.g., 3.5.2), make sure it’s |
noahdietz | 5abf802 | 2022-04-12 10:25:08 -0700 | [diff] [blame] | 59 | already merged to main, and then create a pull request cherry-picking the |
| 60 | commits from main branch to the release branch (e.g., branch 3.5.x). |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 61 | |
| 62 | For each pull request, a protobuf team member will be assigned to review the |
| 63 | pull request. For minor cleanups, the pull request may be merged right away |
| 64 | after an initial review. For larger changes, you will likely receive multiple |
| 65 | rounds of comments and it may take some time to complete. We will try to keep |
| 66 | our response time within 7-days but if you don’t get any response in a few |
| 67 | days, feel free to comment on the threads to get our attention. We also expect |
| 68 | you to respond to our comments within a reasonable amount of time. If we don’t |
| 69 | hear from you for 2 weeks or longer, we may close the pull request. You can |
| 70 | still send the pull request again once you have time to work on it. |
| 71 | |
| 72 | Once a pull request is merged, we will take care of the rest and get it into |
| 73 | the final release. |
| 74 | |
| 75 | ## Pull Request Guidelines |
| 76 | |
| 77 | * If you are a Googler, it is preferable to first create an internal CL and |
| 78 | have it reviewed and submitted. The code propagation process will deliver the |
| 79 | change to GitHub. |
| 80 | * Create small PRs that are narrowly focused on addressing a single concern. |
| 81 | We often receive PRs that are trying to fix several things at a time, but if |
| 82 | only one fix is considered acceptable, nothing gets merged and both author's |
Elliotte Rusty Harold | ab993cf | 2021-10-13 16:36:21 +0000 | [diff] [blame] | 83 | & reviewer's time is wasted. Create more PRs to address different concerns and |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 84 | everyone will be happy. |
| 85 | * For speculative changes, consider opening an issue and discussing it first. |
| 86 | If you are suggesting a behavioral or API change, make sure you get explicit |
| 87 | support from a protobuf team member before sending us the pull request. |
| 88 | * Provide a good PR description as a record of what change is being made and |
| 89 | why it was made. Link to a GitHub issue if it exists. |
| 90 | * Don't fix code style and formatting unless you are already changing that |
| 91 | line to address an issue. PRs with irrelevant changes won't be merged. If |
| 92 | you do want to fix formatting or style, do that in a separate PR. |
| 93 | * Unless your PR is trivial, you should expect there will be reviewer comments |
| 94 | that you'll need to address before merging. We expect you to be reasonably |
| 95 | responsive to those comments, otherwise the PR will be closed after 2-3 weeks |
| 96 | of inactivity. |
| 97 | * Maintain clean commit history and use meaningful commit messages. PRs with |
| 98 | messy commit history are difficult to review and won't be merged. Use rebase |
noahdietz | 5abf802 | 2022-04-12 10:25:08 -0700 | [diff] [blame] | 99 | -i upstream/main to curate your commit history and/or to bring in latest |
| 100 | changes from main (but avoid rebasing in the middle of a code review). |
| 101 | * Keep your PR up to date with upstream/main (if there are merge conflicts, |
Feng Xiao | 2102a70 | 2018-08-29 15:50:16 -0700 | [diff] [blame] | 102 | we can't really merge your change). |
| 103 | * All tests need to be passing before your change can be merged. We recommend |
| 104 | you run tests locally before creating your PR to catch breakages early on. |
| 105 | Ultimately, the green signal will be provided by our testing infrastructure. |
| 106 | The reviewer will help you if there are test failures that seem not related |
| 107 | to the change you are making. |
Hao Nguyen | e06c5ff | 2019-05-31 15:32:02 -0700 | [diff] [blame] | 108 | |
| 109 | ## Reviewer Guidelines |
| 110 | |
| 111 | * Make sure that all tests are passing before approval. |
Hao Nguyen | 9e62059 | 2019-03-18 13:33:08 -0700 | [diff] [blame] | 112 | * Apply the "release notes: yes" label if the pull request's description should |
| 113 | be included in the next release (e.g., any new feature / bug fix). |
| 114 | Apply the "release notes: no" label if the pull request's description should |
| 115 | not be included in the next release (e.g., refactoring changes that does not |
| 116 | change behavior, integration from Google internal, updating tests, etc.). |
| 117 | * Apply the appropriate language label (e.g., C++, Java, Python, etc.) to the |
| 118 | pull request. This will make it easier to identify which languages the pull |
| 119 | request affects, allowing us to better identify appropriate reviewer, create |
| 120 | a better release note, and make it easier to identify issues in the future. |