1# Contributing to Tonic 2 3:balloon: Thanks for your help improving the project! We are so happy to have 4you! 5 6There are opportunities to contribute to `tonic` at any level. It doesn't 7matter if you are just getting started with Rust or are the most weathered 8expert, we can use your help. 9 10**No contribution is too small and all contributions are valued.** 11 12This guide will help you get started. **Do not let this guide intimidate you**. 13It should be considered a map to help you navigate the process. 14 15You may also get help with contributing in the `dev` channel, please join 16us! 17 18Tonic is a part of the [Tokio][tokio] and [Hyperium][hyperium] project, and follows the project's 19guidelines for contributing. This document is based on the 20[`CONTRIBUTING.md` file][tokio-contrib] in the `tokio-rs/tokio` repository. 21 22[dev]: https://gitter.im/tokio-rs/dev 23[tokio]: https://tokio.rs 24[hyperium]: https://github.com/hyperium 25[tokio-contrib]: https://github.com/tokio-rs/tokio/blob/master/CONTRIBUTING.md 26 27## Conduct 28 29The `tonic` project adheres to the [Rust Code of Conduct][coc]. This describes 30the _minimum_ behavior expected from all contributors. 31 32[coc]: https://github.com/rust-lang/rust/blob/master/CODE_OF_CONDUCT.md 33 34## Contributing in Issues 35 36For any issue, there are fundamentally three ways an individual can contribute: 37 381. By opening the issue for discussion: For instance, if you believe that you 39 have uncovered a bug in a `tonic` crate, creating a new issue in the 40 hyperium/tonic [issue tracker][issues] is the way to report it. 41 422. By helping to triage the issue: This can be done by providing 43 supporting details (a test case that demonstrates a bug), providing 44 suggestions on how to address the issue, or ensuring that the issue is tagged 45 correctly. 46 473. By helping to resolve the issue: Typically this is done either in the form of 48 demonstrating that the issue reported is not a problem after all, or more 49 often, by opening a Pull Request that changes some bit of something in 50 Tokio in a concrete and reviewable manner. 51 52**Anybody can participate in any stage of contribution**. We urge you to 53participate in the discussion around bugs and participate in reviewing PRs. 54 55[issues]: https://github.com/hyperium/tonic/issues 56 57### Asking for General Help 58 59If you have reviewed existing documentation and still have questions or are 60having problems, you can open an issue asking for help. 61 62In exchange for receiving help, we ask that you contribute back a documentation 63PR that helps others avoid the problems that you encountered. 64 65### Submitting a Bug Report 66 67When opening a new issue in the `tonic` issue tracker, users will 68be presented with a [basic template][template] that should be filled in. If you 69believe that you have uncovered a bug, please fill out this form, following the 70template to the best of your ability. Do not worry if you cannot answer every 71detail, just fill in what you can. 72 73The two most important pieces of information we need in order to properly 74evaluate the report is a description of the behavior you are seeing and a simple 75test case we can use to recreate the problem on our own. If we cannot recreate 76the issue, it becomes impossible for us to fix. 77 78In order to rule out the possibility of bugs introduced by userland code, test 79cases should be limited, as much as possible, to using only Tokio APIs. 80 81See [How to create a Minimal, Complete, and Verifiable example][mcve]. 82 83[mcve]: https://stackoverflow.com/help/mcve 84[template]: .github/ISSUE_TEMPLATE/bug_report.md 85 86### Triaging a Bug Report 87 88Once an issue has been opened, it is not uncommon for there to be discussion 89around it. Some contributors may have differing opinions about the issue, 90including whether the behavior being seen is a bug or a feature. This discussion 91is part of the process and should be kept focused, helpful, and professional. 92 93Short, clipped responses—that provide neither additional context nor supporting 94detail—are not helpful or professional. To many, such responses are simply 95annoying and unfriendly. 96 97Contributors are encouraged to help one another make forward progress as much as 98possible, empowering one another to solve issues collaboratively. If you choose 99to comment on an issue that you feel either is not a problem that needs to be 100fixed, or if you encounter information in an issue that you feel is incorrect, 101explain why you feel that way with additional supporting context, and be willing 102to be convinced that you may be wrong. By doing so, we can often reach the 103correct outcome much faster. 104 105### Resolving a Bug Report 106 107In the majority of cases, issues are resolved by opening a Pull Request. The 108process for opening and reviewing a Pull Request is similar to that of opening 109and triaging issues, but carries with it a necessary review and approval 110workflow that ensures that the proposed changes meet the minimal quality and 111functional guidelines of the Tokio project. 112 113## Pull Requests 114 115Pull Requests are the way concrete changes are made to the code, documentation, 116and dependencies in the `tonic` repository. 117 118Even tiny pull requests (e.g., one character pull request fixing a typo in API 119documentation) are greatly appreciated. Before making a large change, it is 120usually a good idea to first open an issue describing the change to solicit 121feedback and guidance. This will increase the likelihood of the PR getting 122merged. 123 124### Tests 125 126If the change being proposed alters code (as opposed to only documentation for 127example), it is either adding new functionality to a crate or it is fixing 128existing, broken functionality. In both of these cases, the pull request should 129include one or more tests to ensure that the crate does not regress in the future. 130There are two ways to write tests: integration tests and documentation tests 131(Tokio avoids unit tests as much as possible). 132 133#### Integration tests 134 135Integration tests go in the same crate as the code they are testing. Each sub 136crate should have a `dev-dependency` on `tonic` itself. This makes all 137`tonic` utilities available to use in tests, no matter the crate being 138tested. 139 140The best strategy for writing a new integration test is to look at existing 141integration tests in the crate and follow the style. 142 143#### Documentation tests 144 145Ideally, every API has at least one [documentation test] that demonstrates how to 146use the API. Documentation tests are run with `cargo test --doc`. This ensures 147that the example is correct and provides additional test coverage. 148 149The trick to documentation tests is striking a balance between being succinct 150for a reader to understand and actually testing the API. 151 152The type level example for `tokio_timer::Timeout` provides a good example of a 153documentation test: 154 155```rust 156/// // import the `timeout` function, usually this is done 157/// // with `use tokio::prelude::*` 158/// use tokio::prelude::FutureExt; 159/// use futures::Stream; 160/// use futures::sync::mpsc; 161/// use std::time::Duration; 162/// 163/// # fn main() { 164/// let (tx, rx) = mpsc::unbounded(); 165/// # tx.unbounded_send(()).unwrap(); 166/// # drop(tx); 167/// 168/// let process = rx.for_each(|item| { 169/// // do something with `item` 170/// # drop(item); 171/// # Ok(()) 172/// }); 173/// 174/// # tokio::runtime::current_thread::block_on_all( 175/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds. 176/// process.timeout(Duration::from_millis(10)) 177/// # ).unwrap(); 178/// # } 179``` 180 181Given that this is a *type* level documentation test and the primary way users 182of `tokio` will create an instance of `Timeout` is by using 183`FutureExt::timeout`, this is how the documentation test is structured. 184 185Lines that start with `/// #` are removed when the documentation is generated. 186They are only there to get the test to run. The `block_on_all` function is the 187easiest way to execute a future from a test. 188 189If this were a documentation test for the `Timeout::new` function, then the 190example would explicitly use `Timeout::new`. For example: 191 192```rust 193/// use tokio::timer::Timeout; 194/// use futures::Future; 195/// use futures::sync::oneshot; 196/// use std::time::Duration; 197/// 198/// # fn main() { 199/// let (tx, rx) = oneshot::channel(); 200/// # tx.send(()).unwrap(); 201/// 202/// # tokio::runtime::current_thread::block_on_all( 203/// // Wrap the future with a `Timeout` set to expire in 10 milliseconds. 204/// Timeout::new(rx, Duration::from_millis(10)) 205/// # ).unwrap(); 206/// # } 207``` 208 209#### Generated code 210 211When making changes to `tonic-build` that affects the generated code you will 212need to ensure that each of the sub crates gets updated as well. Each of the sub 213crates like, for example `tonic-health`, generate their gRPC code via `codegen` 214crate. 215 216``` 217cargo run --package codegen 218``` 219 220### Commits 221 222It is a recommended best practice to keep your changes as logically grouped as 223possible within individual commits. There is no limit to the number of commits 224any single Pull Request may have, and many contributors find it easier to review 225changes that are split across multiple commits. 226 227That said, if you have a number of commits that are "checkpoints" and don't 228represent a single logical change, please squash those together. 229 230Note that multiple commits often get squashed when they are landed (see the 231notes about [commit squashing]). 232 233#### Commit message guidelines 234 235A good commit message should describe what changed and why. 236 2371. The first line should: 238 239 * contain a short description of the change (preferably 50 characters or less, 240 and no more than 72 characters) 241 * be entirely in lowercase with the exception of proper nouns, acronyms, and 242 the words that refer to code, like function/variable names 243 * be prefixed with the name of the crate being changed (without the 244 `tonic` prefix) and start with an imperative verb. 245 246 Examples: 247 248 * build: add regex for parsing field filters 249 * tonic: add `Clone` impl for `Service` and `MakeService` 250 2512. Keep the second line blank. 2523. Wrap all other lines at 72 columns (except for long URLs). 2534. If your patch fixes an open issue, you can add a reference to it at the end 254 of the log. Use the `Fixes: #` prefix and the issue number. For other 255 references use `Refs: #`. `Refs` may include multiple issues, separated by a 256 comma. 257 258 Examples: 259 260 - `Fixes: #1337` 261 - `Refs: #1234` 262 263Sample complete commit message: 264 265```txt 266subcrate: explain the commit in one line 267 268Body of commit message is a few lines of text, explaining things 269in more detail, possibly giving some background about the issue 270being fixed, etc. 271 272The body of the commit message can be several paragraphs, and 273please do proper word-wrap and keep columns shorter than about 27472 characters or so. That way, `git log` will show things 275nicely even when it is indented. 276 277Fixes: #1337 278Refs: #453, #154 279``` 280 281### Opening the Pull Request 282 283From within GitHub, opening a new Pull Request will present you with a 284[template] that should be filled out. Please try to do your best at filling out 285the details, but feel free to skip parts if you're not sure what to put. 286 287[template]: .github/PULL_REQUEST_TEMPLATE.md 288 289### Discuss and update 290 291You will probably get feedback or requests for changes to your Pull Request. 292This is a big part of the submission process so don't be discouraged! Some 293contributors may sign off on the Pull Request right away, others may have 294more detailed comments or feedback. This is a necessary part of the process 295in order to evaluate whether the changes are correct and necessary. 296 297**Any community member can review a PR and you might get conflicting feedback**. 298Keep an eye out for comments from code owners to provide guidance on conflicting 299feedback. 300 301**Once the PR is open, do not rebase the commits**. See [Commit Squashing] for 302more details. 303 304### Commit Squashing 305 306In most cases, **do not squash commits that you add to your Pull Request during 307the review process**. When the commits in your Pull Request land, they may be 308squashed into one commit per logical change. Metadata will be added to the 309commit message (including links to the Pull Request, links to relevant issues, 310and the names of the reviewers). The commit history of your Pull Request, 311however, will stay intact on the Pull Request page. 312 313## Reviewing Pull Requests 314 315**Any Tokio and Hyperium community member is welcome to review any pull request**. 316 317All Tokio contributors who choose to review and provide feedback on Pull 318Requests have a responsibility to both the project and the individual making the 319contribution. Reviews and feedback must be helpful, insightful, and geared 320towards improving the contribution as opposed to simply blocking it. If there 321are reasons why you feel the PR should not land, explain what those are. Do not 322expect to be able to block a Pull Request from advancing simply because you say 323"No" without giving an explanation. Be open to having your mind changed. Be open 324to working with the contributor to make the Pull Request better. 325 326Reviews that are dismissive or disrespectful of the contributor or any other 327reviewers are strictly counter to the Code of Conduct. 328 329When reviewing a Pull Request, the primary goals are for the codebase to improve 330and for the person submitting the request to succeed. **Even if a Pull Request 331does not land, the submitters should come away from the experience feeling like 332their effort was not wasted or unappreciated**. Every Pull Request from a new 333contributor is an opportunity to grow the community. 334 335### Review a bit at a time. 336 337Do not overwhelm new contributors. 338 339It is tempting to micro-optimize and make everything about relative performance, 340perfect grammar, or exact style matches. Do not succumb to that temptation. 341 342Focus first on the most significant aspects of the change: 343 3441. Does this change make sense for Tokio? 3452. Does this change make Tokio better, even if only incrementally? 3463. Are there clear bugs or larger scale issues that need attending to? 3474. Is the commit message readable and correct? If it contains a breaking change 348 is it clear enough? 349 350Note that only **incremental** improvement is needed to land a PR. This means 351that the PR does not need to be perfect, only better than the status quo. Follow 352up PRs may be opened to continue iterating. 353 354When changes are necessary, *request* them, do not *demand* them, and **do not 355assume that the submitter already knows how to add a test or run a benchmark**. 356 357Specific performance optimization techniques, coding styles and conventions 358change over time. The first impression you give to a new contributor never does. 359 360Nits (requests for small changes that are not essential) are fine, but try to 361avoid stalling the Pull Request. Most nits can typically be fixed by the Tokio 362Collaborator landing the Pull Request but they can also be an opportunity for 363the contributor to learn a bit more about the project. 364 365It is always good to clearly indicate nits when you comment: e.g. 366`Nit: change foo() to bar(). But this is not blocking.` 367 368If your comments were addressed but were not folded automatically after new 369commits or if they proved to be mistaken, please, [hide them][hiding-a-comment] 370with the appropriate reason to keep the conversation flow concise and relevant. 371 372### Be aware of the person behind the code 373 374Be aware that *how* you communicate requests and reviews in your feedback can 375have a significant impact on the success of the Pull Request. Yes, we may land 376a particular change that makes `tonic` better, but the individual might 377just not want to have anything to do with `tonic` ever again. The goal is 378not just having good code. 379 380### Abandoned or Stalled Pull Requests 381 382If a Pull Request appears to be abandoned or stalled, it is polite to first 383check with the contributor to see if they intend to continue the work before 384checking if they would mind if you took it over (especially if it just has nits 385left). When doing so, it is courteous to give the original contributor credit 386for the work they started (either by preserving their name and email address in 387the commit log, or by using an `Author: ` meta-data tag in the commit. 388 389_Adapted from the [Node.js contributing guide][node]_. 390 391[node]: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md 392[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment 393[documentation test]: https://doc.rust-lang.org/rustdoc/documentation-tests.html 394 395## Releasing 396 397Since the Tonic project consists of a number of crates, many of which depend on 398each other, releasing new versions to crates.io can involve some complexities. 399When releasing a new version of a crate, follow these steps: 400 4011. First you must pick the correct version to release, if there are breaking 402 changes make sure to select a semver compatible version bump. 403 4042. In general, tonic tries to keep all crates at the same version to make it 405 easy to release and figure out what sub crates you need that will work with 406 the core version of tonic. To prepare a release branch you can follow the 407 commands below: 408 ``` 409 git checkout -b <release-branch-name> 410 ./prepare-release.sh <version> # where version is X.Y.Z 411 ``` 412 4133. Once all the crate versions have been updated its time to update the 414 changelog. Tonic uses `conventional-changelog` and it's cli to generate the 415 changelog. 416 417 ``` 418 conventional-changelog -p angular -i CHANGELOG.md -s 419 ``` 420 421 Once the entries have been generated, you must edit the `CHANGELOG.md` file 422 to add the version and tag to the title and edit any changelog entries. You 423 must also add any breaking changes here as sometimes they get lost. 424 4254. Once the changelog has been updated you can now create a `chore: release 426 vX.Y.Z` commit and push the release branch and open a release PR. 427 4285. Once the release PR has been approved and merged into `master` the following 429 command will release those changes. 430 431 ``` 432 ./publish-release.sh 433 ``` 434 4356. Once all the crates have been released you now must create a release on 436 github using the text from the changelog. 437 438