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