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