|
| 1 | +# Contributing to tracing-opentelemetry |
| 2 | + |
| 3 | +🎈 Thanks for your help improving the project! We are so happy to have |
| 4 | +you! |
| 5 | + |
| 6 | +There are opportunities to contribute to `tracing-opetelemetry` at any level. It |
| 7 | +doesn't matter if you are just getting started with Rust or are the most |
| 8 | +weathered expert, we can use your help. |
| 9 | + |
| 10 | +**No contribution is too small and all contributions are valued.** |
| 11 | + |
| 12 | +This guide will help you get started. **Do not let this guide intimidate you**. |
| 13 | +It should be considered a map to help you navigate the process. |
| 14 | + |
| 15 | +You may also get help with contributing in the [`tokio` Discord |
| 16 | +channel][discord], please join us! |
| 17 | + |
| 18 | +[discord]: https://discord.gg/tokio |
| 19 | + |
| 20 | +## Conduct |
| 21 | + |
| 22 | +The `tracing-opentelemetry` project adheres to the [Rust Code of Conduct][coc]. |
| 23 | +This describes the _minimum_ behavior expected from all contributors. |
| 24 | + |
| 25 | +[coc]: https://github.com/rust-lang/rust/blob/master/CODE_OF_CONDUCT.md |
| 26 | + |
| 27 | +## Contributing in Issues |
| 28 | + |
| 29 | +For any issue, there are fundamentally three ways an individual can contribute: |
| 30 | + |
| 31 | +1. By opening the issue for discussion: For instance, if you believe that you |
| 32 | + have uncovered a bug in a `tracing-opentelemetry` crate, creating a new issue in the |
| 33 | + tokio-rs/tracing-opentelemetry [issue tracker][issues] is the way to report it. |
| 34 | + |
| 35 | +2. By helping to triage the issue: This can be done by providing |
| 36 | + supporting details (a test case that demonstrates a bug), providing |
| 37 | + suggestions on how to address the issue, or ensuring that the issue is tagged |
| 38 | + correctly. |
| 39 | + |
| 40 | +3. By helping to resolve the issue: Typically this is done either in the form of |
| 41 | + demonstrating that the issue reported is not a problem after all, or more |
| 42 | + often, by opening a Pull Request that changes some bit of something in |
| 43 | + `tracing-opentelemetry` in a concrete and reviewable manner. |
| 44 | + |
| 45 | +**Anybody can participate in any stage of contribution**. We urge you to |
| 46 | +participate in the discussion around bugs and participate in reviewing PRs. |
| 47 | + |
| 48 | +[issues]: https://github.com/tokio-rs/tracing-opentelemetry/issues |
| 49 | + |
| 50 | +### Asking for General Help |
| 51 | + |
| 52 | +If you have reviewed existing documentation and still have questions or are |
| 53 | +having problems, you can open an issue asking for help. |
| 54 | + |
| 55 | +In exchange for receiving help, we ask that you contribute back a documentation |
| 56 | +PR that helps others avoid the problems that you encountered. |
| 57 | + |
| 58 | +### Submitting a Bug Report |
| 59 | + |
| 60 | +When opening a new issue in the `tracing-opentelemetry` issue tracker, users will |
| 61 | +be presented with a [basic template][template] that should be filled in. If you |
| 62 | +believe that you have uncovered a bug, please fill out this form, following the |
| 63 | +template to the best of your ability. Do not worry if you cannot answer every |
| 64 | +detail, just fill in what you can. |
| 65 | + |
| 66 | +The two most important pieces of information we need in order to properly |
| 67 | +evaluate the report is a description of the behavior you are seeing and a simple |
| 68 | +test case we can use to recreate the problem on our own. If we cannot recreate |
| 69 | +the issue, it becomes harder for us to fix. |
| 70 | + |
| 71 | +See [How to create a Minimal, Complete, and Verifiable example][mcve]. |
| 72 | + |
| 73 | +[mcve]: https://stackoverflow.com/help/mcve |
| 74 | +[template]: .github/ISSUE_TEMPLATE/bug_report.md |
| 75 | + |
| 76 | +### Triaging a Bug Report |
| 77 | + |
| 78 | +Once an issue has been opened, it is not uncommon for there to be discussion |
| 79 | +around it. Some contributors may have differing opinions about the issue, |
| 80 | +including whether the behavior being seen is a bug or a feature. This discussion |
| 81 | +is part of the process and should be kept focused, helpful, and professional. |
| 82 | + |
| 83 | +Short, clipped responses—that provide neither additional context nor supporting |
| 84 | +detail—are not helpful or professional. To many, such responses are simply |
| 85 | +annoying and unfriendly. |
| 86 | + |
| 87 | +Contributors are encouraged to help one another make forward progress as much as |
| 88 | +possible, empowering one another to solve issues collaboratively. If you choose |
| 89 | +to comment on an issue that you feel either is not a problem that needs to be |
| 90 | +fixed, or if you encounter information in an issue that you feel is incorrect, |
| 91 | +explain why you feel that way with additional supporting context, and be willing |
| 92 | +to be convinced that you may be wrong. By doing so, we can often reach the |
| 93 | +correct outcome much faster. |
| 94 | + |
| 95 | +### Resolving a Bug Report |
| 96 | + |
| 97 | +In the majority of cases, issues are resolved by opening a Pull Request. The |
| 98 | +process for opening and reviewing a Pull Request is similar to that of opening |
| 99 | +and triaging issues, but carries with it a necessary review and approval |
| 100 | +workflow that ensures that the proposed changes meet the minimal quality. |
| 101 | + |
| 102 | +## Pull Requests |
| 103 | + |
| 104 | +Pull Requests are the way concrete changes are made to the code, documentation, |
| 105 | +and dependencies in the `tracing-opentelemetry` repository. |
| 106 | + |
| 107 | +Even tiny pull requests (e.g., one character pull request fixing a typo in API |
| 108 | +documentation) are greatly appreciated. Before making a large change, it is |
| 109 | +usually a good idea to first open an issue describing the change to solicit |
| 110 | +feedback and guidance. This will increase the likelihood of the PR getting |
| 111 | +merged. |
| 112 | + |
| 113 | +### Tests |
| 114 | + |
| 115 | +If the change being proposed alters code (as opposed to only documentation for |
| 116 | +example), it is either adding new functionality to a crate or it is fixing |
| 117 | +existing, broken functionality. In both of these cases, the pull request should |
| 118 | +include one or more tests to ensure that the crate does not regress in the future. |
| 119 | + |
| 120 | +#### Documentation tests |
| 121 | + |
| 122 | +Ideally, every API has at least one [documentation test] that demonstrates how to |
| 123 | +use the API. Documentation tests are run with `cargo test --doc`. This ensures |
| 124 | +that the example is correct and provides additional test coverage. |
| 125 | + |
| 126 | +The trick to documentation tests is striking a balance between being succinct |
| 127 | +for a reader to understand and actually testing the API. |
| 128 | + |
| 129 | +In Rust documentation, lines that start with `/// #` are removed when the |
| 130 | +documentation is generated. They are only there to get the test to run. |
| 131 | + |
| 132 | +### Commits |
| 133 | + |
| 134 | +It is a recommended best practice to keep your changes as logically grouped as |
| 135 | +possible within individual commits. There is no limit to the number of commits |
| 136 | +any single Pull Request may have, and many contributors find it easier to review |
| 137 | +changes that are split across multiple commits. |
| 138 | + |
| 139 | +Note that multiple commits often get squashed when they are landed (see the |
| 140 | +notes about [commit squashing]). |
| 141 | + |
| 142 | +#### Commit message guidelines |
| 143 | + |
| 144 | +A good commit message should describe what changed and why. |
| 145 | + |
| 146 | +1. The first line should: |
| 147 | + |
| 148 | + * Contain a short description of the change (preferably 50 characters or less, |
| 149 | + and no more than 72 characters) |
| 150 | + |
| 151 | +2. Keep the second line blank. |
| 152 | +3. Wrap all other lines at 72 columns (except for long URLs). |
| 153 | +4. If your patch fixes an open issue, you can add a reference to it at the end |
| 154 | + of the log. Use the `Fixes: #` prefix and the issue number. For other |
| 155 | + references use `Refs: #`. `Refs` may include multiple issues, separated by a |
| 156 | + comma. |
| 157 | + |
| 158 | + Examples: |
| 159 | + |
| 160 | + - `Fixes: #1337` |
| 161 | + - `Refs: #1234, #42` |
| 162 | + |
| 163 | +### Opening the Pull Request |
| 164 | + |
| 165 | +From within GitHub, opening a new Pull Request will present you with a |
| 166 | +[template] that should be filled out. Please try to do your best at filling out |
| 167 | +the details, but feel free to skip parts if you're not sure what to put. |
| 168 | + |
| 169 | +[template]: .github/PULL_REQUEST_TEMPLATE.md |
| 170 | + |
| 171 | +### Discuss and update |
| 172 | + |
| 173 | +You will probably get feedback or requests for changes to your Pull Request. |
| 174 | +This is a big part of the submission process so don't be discouraged! Some |
| 175 | +contributors may sign off on the Pull Request right away, others may have |
| 176 | +more detailed comments or feedback. This is a necessary part of the process |
| 177 | +in order to evaluate whether the changes are correct and necessary. |
| 178 | + |
| 179 | +**Any community member can review a PR and you might get conflicting feedback**. |
| 180 | +Keep an eye out for comments from code owners to provide guidance on conflicting |
| 181 | +feedback. |
| 182 | + |
| 183 | +**Once the PR is open, do not rebase the commits**. See [Commit Squashing] for |
| 184 | +more details. |
| 185 | + |
| 186 | +### Commit Squashing |
| 187 | + |
| 188 | +In most cases, **do not squash commits that you add to your Pull Request during |
| 189 | +the review process**. When the commits in your Pull Request land, they may be |
| 190 | +squashed into one commit per logical change. Metadata will be added to the |
| 191 | +commit message (including links to the Pull Request, links to relevant issues, |
| 192 | +and the names of the reviewers). The commit history of your Pull Request, |
| 193 | +however, will stay intact on the Pull Request page. |
| 194 | + |
| 195 | +## Reviewing Pull Requests |
| 196 | + |
| 197 | +**Community member is welcome to review any pull request**. |
| 198 | + |
| 199 | +All contributors who choose to review and provide feedback on Pull Requests have |
| 200 | +a responsibility to both the project and the individual making the contribution. |
| 201 | +Reviews and feedback must be helpful, insightful, and geared towards improving |
| 202 | +the contribution as opposed to simply blocking it. If there are reasons why you |
| 203 | +feel the PR should not land, explain what those are. Do not expect to be able to |
| 204 | +block a Pull Request from advancing simply because you say "No" without giving |
| 205 | +an explanation. Be open to having your mind changed. Be open to working with the |
| 206 | +contributor to make the Pull Request better. |
| 207 | + |
| 208 | +Reviews that are dismissive or disrespectful of the contributor or any other |
| 209 | +reviewers are strictly counter to the Code of Conduct. |
| 210 | + |
| 211 | +When reviewing a Pull Request, the primary goals are for the codebase to improve |
| 212 | +and for the person submitting the request to succeed. **Even if a Pull Request |
| 213 | +does not land, the submitters should come away from the experience feeling like |
| 214 | +their effort was not wasted or unappreciated**. Every Pull Request from a new |
| 215 | +contributor is an opportunity to grow the community. |
| 216 | + |
| 217 | +### Review a bit at a time. |
| 218 | + |
| 219 | +Do not overwhelm new contributors. |
| 220 | + |
| 221 | +It is tempting to micro-optimize and make everything about relative performance, |
| 222 | +perfect grammar, or exact style matches. Do not succumb to that temptation. |
| 223 | + |
| 224 | +Focus first on the most significant aspects of the change: |
| 225 | + |
| 226 | +1. Does this change make sense for `tracing-opentelemetry`? |
| 227 | +2. Does this change make `tracing-opentelemetry` better, even if only incrementally? |
| 228 | +3. Are there clear bugs or larger scale issues that need attending to? |
| 229 | +4. Is the commit message readable and correct? If it contains a breaking change |
| 230 | + is it clear enough? |
| 231 | + |
| 232 | +Note that only **incremental** improvement is needed to land a PR. This means |
| 233 | +that the PR does not need to be perfect, only better than the status quo. Follow |
| 234 | +up PRs may be opened to continue iterating. |
| 235 | + |
| 236 | +When changes are necessary, *request* them, do not *demand* them, and **do not |
| 237 | +assume that the submitter already knows how to add a test or run a benchmark**. |
| 238 | + |
| 239 | +Specific performance optimization techniques, coding styles and conventions |
| 240 | +change over time. The first impression you give to a new contributor never does. |
| 241 | + |
| 242 | +Nits (requests for small changes that are not essential) are fine, but try to |
| 243 | +avoid stalling the Pull Request. Most nits can typically be fixed by the |
| 244 | +collaborator landing the Pull Request but they can also be an opportunity for |
| 245 | +the contributor to learn a bit more about the project. |
| 246 | + |
| 247 | +It is always good to clearly indicate nits when you comment: e.g. |
| 248 | +`Nit: change foo() to bar(). But this is not blocking.` |
| 249 | + |
| 250 | +If your comments were addressed but were not folded automatically after new |
| 251 | +commits or if they proved to be mistaken, please, [hide them][hiding-a-comment] |
| 252 | +with the appropriate reason to keep the conversation flow concise and relevant. |
| 253 | + |
| 254 | +### Be aware of the person behind the code |
| 255 | + |
| 256 | +Be aware that *how* you communicate requests and reviews in your feedback can |
| 257 | +have a significant impact on the success of the Pull Request. Yes, we may land a |
| 258 | +particular change that makes `tracing-opentelemetry` better, but the individual might just |
| 259 | +not want to have anything to do with `tracing-opentelemetry` ever again. The goal is not |
| 260 | +just having good code. |
| 261 | + |
| 262 | +### Abandoned or Stalled Pull Requests |
| 263 | + |
| 264 | +If a Pull Request appears to be abandoned or stalled, it is polite to first |
| 265 | +check with the contributor to see if they intend to continue the work before |
| 266 | +checking if they would mind if you took it over (especially if it just has nits |
| 267 | +left). When doing so, it is courteous to give the original contributor credit |
| 268 | +for the work they started, either by preserving their name and email address in |
| 269 | +the commit log, or by using an `Author: ` meta-data tag in the commit. |
| 270 | + |
| 271 | +[hiding-a-comment]: https://help.github.com/articles/managing-disruptive-comments/#hiding-a-comment |
| 272 | +[documentation test]: https://doc.rust-lang.org/rustdoc/documentation-tests.html |
| 273 | +[keep-a-changelog]: https://github.com/olivierlacan/keep-a-changelog/blob/master/CHANGELOG.md |
0 commit comments