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