xref: /tonic/CONTRIBUTING.md (revision 6d93c1d0)
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