|
| 1 | +# Contribution guidelines |
| 2 | + |
| 3 | +Thank you for your interest in contributing to the `ibc-proto-rs` project! |
| 4 | +The goal of this project is maintain all the data structures relevant for interacting with on-chain IBC data. |
| 5 | + |
| 6 | +All work on the code base should be motivated by a GitHub issue. |
| 7 | +Before opening a new issue, first do a search of open and closed issues to make sure that yours will not be a duplicate. |
| 8 | +If you would like to work on an issue which already exists, please indicate so by leaving a comment on the issue. |
| 9 | +If what you'd like to work on hasn't already been covered by an issue, then open a new one to get the process going. |
| 10 | + |
| 11 | +The rest of this document outlines the best practices for contributing to this repository: |
| 12 | + |
| 13 | +- [Decision Making](#decision-making) - process for agreeing to changes |
| 14 | +- [Issues](#issues) - what makes a good issue |
| 15 | +- [Pull Requests](#pull-requests) - what makes a good pull request |
| 16 | +- [Forking](#forking) - fork the repo to make pull requests |
| 17 | +- [Changelog](#changelog) - changes must be recorded in the changelog |
| 18 | +- [Releases](#releases) - how to release new version of the crates |
| 19 | + |
| 20 | +## Decision Making |
| 21 | + |
| 22 | +When contributing to the project, the following process leads to the best chance of landing the changes in `main`. |
| 23 | + |
| 24 | +All new contributions should start with a GitHub issue which captures the |
| 25 | +problem you're trying to solve. Starting off with an issue allows for early feedback. |
| 26 | + |
| 27 | +When the problem and the proposed solution are well understood, |
| 28 | +changes should start with a [draft pull request](https://github.blog/2019-02-14-introducing-draft-pull-requests/) |
| 29 | +against the branch `main`. The draft status signals that work is underway. |
| 30 | +When the work is ready for feedback, hitting "Ready for Review" will signal to the maintainers to take a look. |
| 31 | + |
| 32 | +Implementation trajectories should aim to proceed where possible as a series |
| 33 | +of smaller incremental changes, in the form of small PRs that can be merged |
| 34 | +quickly. This helps manage the load for reviewers and reduces the likelihood |
| 35 | +that PRs will sit open for long periods of time. |
| 36 | + |
| 37 | +## Issues |
| 38 | + |
| 39 | +We welcome bug reports, feature requests, and other contributions to our project. |
| 40 | +To open an issue, please follow these guidelines: |
| 41 | + |
| 42 | +1. **Search existing issues**: Before opening a new issue, please search existing issues to ensure that is not a duplicates. |
| 43 | + |
| 44 | +2. **Provide a clear and descriptive title**: This helps others understand the nature of the issue at a glance. |
| 45 | + |
| 46 | +3. **Provide detailed information**: In the issue description, clearly state the **purpose** of the issue |
| 47 | + include as much information as possible, such as: |
| 48 | + - Steps to reproduce the issue |
| 49 | + - Expected behavior |
| 50 | + - Actual behavior |
| 51 | + - The version of the operating system and the software you are using |
| 52 | + - Error messages or logs (if applicable) |
| 53 | + |
| 54 | +This assist us prioritize and categorize your issue more effectively and help |
| 55 | +others and reviewers understand the type and severity of the issue. |
| 56 | +If the issue you worked on was tagged `A: low-priority`, we'll do our best to |
| 57 | +review it in a timely manner, but please expect longer wait times for a review |
| 58 | +in general. If a low priority issue is important to you, please leave a comment |
| 59 | +explaining why, and we will reprioritize it! |
| 60 | + |
| 61 | +## Pull Requests |
| 62 | + |
| 63 | +If you have write access to the ibc-proto-rs repo, you can directly branch off of `main`. |
| 64 | +This makes it easier for project maintainers to directly make changes to your |
| 65 | +branch should the need arise. Otherwise, check the [Forking](#forking) section for instructions. |
| 66 | + |
| 67 | +Branch names should be prefixed with the author's name followed by a short description of the feature, eg. `name/feature-x`. |
| 68 | + |
| 69 | +Pull requests are made against `main` and are squash-merged into it. Each PR should: |
| 70 | +- make reference to an issue outlining the context |
| 71 | +- update any relevant documentation and include tests |
| 72 | +- add a corresponding entry in the `.changelog` directory using `unclog`, |
| 73 | + see the [Changelog](#changelog) section for more details. |
| 74 | + |
| 75 | +Additionally, in order to make PRs as easy to review as possible, each PR should: |
| 76 | +- Be focused on implementing _*one*_ piece of logic from end-to-end. |
| 77 | + It must be very clear what the purpose of the PR is from looking at the PR's title, |
| 78 | + description, and/or linked issue(s). |
| 79 | + It should also be very clear what value the changes incorporated in the PR aim to deliver. |
| 80 | + A single PR that does multiple things, without a clear articulation of the problem it attempts to solve, will very likely be rejected. |
| 81 | +- Be small, ideally no more than 500 lines of code changes. |
| 82 | + While this is a guideline and not a hard rule, in general, larger changes should being |
| 83 | + structured as a series of PRs, each building off of the previous ones; |
| 84 | + these PRs should also be tracked in a tracking issue. |
| 85 | + If a single PR absolutely has to be larger, it _must_ be structured such that |
| 86 | + it can be reviewed commit-by-commit, with each commit doing a single logical thing, |
| 87 | + accompanied with a good description of what it aims to achieve in the git commit message. |
| 88 | + Poorly structured PRs will likely be rejected on the grounds of being too much of |
| 89 | + a burden for the core maintainers to review; you will be asked to restructure the |
| 90 | + PR in accordance with the guidelines laid out here. |
| 91 | + This does not necessarily apply to documentation-related changes or |
| 92 | + automatically generated code (e.g. generated from Protobuf definitions). |
| 93 | + But automatically generated code changes should occur within separate commits, |
| 94 | + so they are easily distinguishable from manual code changes. |
| 95 | + |
| 96 | +In order to help facilitate the PR review process, tag *one* person as the |
| 97 | +reviewer of the PR. If you are unsure of who to tag, your point of contact on |
| 98 | +the ibc-proto-rs team is always a natural candidate; they'll make sure that the PR gets |
| 99 | +reviewed by whomever is most appropriate to review it. It also helps to notify |
| 100 | +the person whom you tagged as reviewer through direct means, such as through |
| 101 | +Slack or Discord, as it is easy for GitHub notifications to get lost or buried. |
| 102 | + |
| 103 | +## Responsibilities of a PR Reviewer |
| 104 | + |
| 105 | +If you're tagged as the reviewer of a PR, you are responsible for shepherding it |
| 106 | +through to completion. This includes fixing issues with the PR and taking the |
| 107 | +lead on decisions that need to be resolved in order to get the PR merged. |
| 108 | + |
| 109 | +If you're tagged as a reviewer on a PR that affects a part of the code base that |
| 110 | +you are unfamiliar with, you can hand it off to someone (with their consent) |
| 111 | +who is more appropriate to shepherd the PR through to completion. |
| 112 | + |
| 113 | +## Forking |
| 114 | + |
| 115 | +If you do not have write access to the repository, your contribution should be |
| 116 | +made through a fork on GitHub. Fork the repository, contribute to your fork |
| 117 | +(either in the `main` branch of the fork or in a separate branch), and then |
| 118 | +make a pull request back upstream. |
| 119 | + |
| 120 | +When forking, add your fork's URL as a new git remote in your local copy of the |
| 121 | +repo. For instance, to create a fork and work on a branch of it: |
| 122 | +- Create the fork on GitHub, using the fork button. |
| 123 | +- `cd` to the original clone of the repo on your machine |
| 124 | +- `git remote rename origin upstream` |
| 125 | +- `git remote add origin [email protected]:<location of fork>` |
| 126 | + |
| 127 | +Now `origin` refers to your fork and `upstream` refers to the original version. |
| 128 | +Now `git push -u origin main` to update the fork, and make pull requests |
| 129 | +against the original repo. |
| 130 | + |
| 131 | +To pull in updates from the origin repo, run `git fetch upstream` followed by |
| 132 | +`git rebase upstream/main` (or whatever branch you're working in). |
| 133 | + |
| 134 | +## Changelog |
| 135 | + |
| 136 | +Every non-trivial PR must update the [CHANGELOG](CHANGELOG.md). This is |
| 137 | +accomplished indirectly by adding entries to the `.changelog` folder in |
| 138 | +[`unclog`](https://github.com/informalsystems/unclog) format using the `unclog` CLI tool. |
| 139 | +`CHANGELOG.md` will be built by whomever is responsible for performing a release just |
| 140 | +prior to release - this is to avoid changelog conflicts prior to releases. |
| 141 | + |
| 142 | +### Install `unclog` |
| 143 | + |
| 144 | +```bash |
| 145 | +cargo install unclog |
| 146 | +``` |
| 147 | + |
| 148 | +### Examples |
| 149 | + |
| 150 | +Add a `.changelog` entry to signal that a bug was fixed, without mentioning any component. |
| 151 | + |
| 152 | +```bash |
| 153 | +unclog add -i update-unclog-instructions -s bug -n 1634 -m "Update CONTRIBUTING.md for latest version of unclog" --editor vim |
| 154 | +``` |
| 155 | + |
| 156 | +Add a .changelog entry under the `FEATURES` section in CHANGELOG.md. |
| 157 | + |
| 158 | +```bash |
| 159 | +unclog add -s features --id a-new-feature --issue-no 1235 -m "msg about this new-feature" --editor vim |
| 160 | +``` |
| 161 | + |
| 162 | +### Preview unreleased changes |
| 163 | + |
| 164 | +```bash |
| 165 | +unclog build -u |
| 166 | +``` |
| 167 | + |
| 168 | +The Changelog is *not* a record of what Pull Requests were merged; |
| 169 | +the commit history already shows that. The Changelog is a notice to users |
| 170 | +about how their expectations of the software should be modified. |
| 171 | +It is part of the UX of a release and is a *critical* user facing integration point. |
| 172 | +The Changelog must be clean, inviting, and readable, with concise, meaningful entries. |
| 173 | +Entries must be semantically meaningful to users. If a change takes multiple |
| 174 | +Pull Requests to complete, it should likely have only a single entry in the |
| 175 | +Changelog describing the net effect to the user. Instead of linking PRs directly, we |
| 176 | +instead prefer to log issues, which tend to be higher-level, hence more relevant for users. |
| 177 | + |
| 178 | +When writing Changelog entries, ensure they are targeting users of the software, |
| 179 | +not fellow developers. Developers have much more context and care about more |
| 180 | +things than users do. Changelogs are for users. |
| 181 | + |
| 182 | +Changelog structure is modeled after |
| 183 | +[Tendermint Core](https://github.com/tendermint/tendermint/blob/master/CHANGELOG.md) |
| 184 | +and [Hashicorp Consul](http://github.com/hashicorp/consul/tree/master/CHANGELOG.md). |
| 185 | +See those changelogs for examples. |
| 186 | + |
| 187 | +We currently split changes for a given release between these four sections: |
| 188 | +Breaking Changes, Features, Improvements, and Bug Fixes. |
| 189 | + |
| 190 | +Entries in the changelog should initially be logged in the __Unreleased__ section, which |
| 191 | +represents a "staging area" for accumulating all the changes throughout a |
| 192 | +release (see [Pull Requests](#pull-requests) below). With each release, |
| 193 | +the entries then move from this section into their permanent place under a |
| 194 | +specific release number in Changelog. |
| 195 | + |
| 196 | +Changelog entries should be formatted as follows: |
| 197 | + |
| 198 | +```md |
| 199 | +- Some description about the change ([#xxx](https://github.com/cosmos/ibc-proto-rs/issues/xxx)) (optional @contributor) |
| 200 | +``` |
| 201 | + |
| 202 | +Here `xxx` is the issue number, and `contributor` is the author/s of the change. |
| 203 | + |
| 204 | +It's also acceptable for `xxx` to refer to the relevant pull request, but issue numbers are preferred. |
| 205 | +Note this means issues (or pull-requests) should be opened first so the changelog can then |
| 206 | +be updated with the corresponding number. |
| 207 | + |
| 208 | +Changelog entries should be ordered alphabetically numerically according to their issue/PR number. |
| 209 | + |
| 210 | +Changes with multiple classifications should be doubly included (eg. a bug fix |
| 211 | +that is also a breaking change should be recorded under both). |
| 212 | + |
| 213 | +Breaking changes are further subdivided according to the APIs/users they impact. |
| 214 | +Any change that effects multiple APIs/users should be recorded multiply - for |
| 215 | +instance, a change to some core protocol data structure might need to be |
| 216 | +reflected both as breaking the core protocol but also breaking any APIs where core data structures are |
| 217 | +exposed. |
| 218 | + |
| 219 | +## Releases |
| 220 | + |
| 221 | +Our release process is as follows: |
| 222 | + |
| 223 | +1. Update the [changelog](#changelog) to reflect and summarize all changes in |
| 224 | + the release. This involves: |
| 225 | + 1. Running `unclog build -u` and copy pasting the output at the top |
| 226 | + of the `CHANGELOG.md` file, making sure to update the header with |
| 227 | + the new version. |
| 228 | + 1. Running `unclog release vX.Y.Z` to create a summary of all of the changes |
| 229 | + in this release. |
| 230 | + 3. Committing the updated `CHANGELOG.md` file and `.changelog` directory to the repo. |
| 231 | +2. Push this to a branch `release/vX.Y.Z` according to the version number of |
| 232 | + the anticipated release (e.g. `release/v0.18.0`) and open a **draft PR**. |
| 233 | +3. Bump all relevant versions in the codebase to the new version and push these |
| 234 | + changes to the release PR. This includes: |
| 235 | + 1. All `Cargo.toml` files (making sure dependencies' versions are updated too). |
| 236 | + 2. All crates' `lib.rs` files documentation references' `html_root_url` |
| 237 | + parameters must point to the new version. |
| 238 | +4. Run `cargo doc --all-features --open` locally to double-check that all the |
| 239 | + documentation compiles and is up-to-date and coherent. Fix any potential |
| 240 | + issues here and push them to the release PR. |
| 241 | +5. Mark the PR as **Ready for Review** and incorporate feedback on the release. |
| 242 | +6. Once approved, merge the PR. |
| 243 | +7. Checkout the `main` and pull it with `git checkout main && git pull origin main`. |
| 244 | + Then create a signed tag and push it to GitHub: `git tag -s -a vX.Y.Z && git push origin vX.Y.Z` |
| 245 | + In the tag message, write the version and the link to the corresponding section of the changelog. |
| 246 | +9. If any problem arises, submit a new PR, get it merged to `main` and try again. |
| 247 | + The reason for not releasing straight from the release branch, and therefore losing the |
| 248 | + ability to fix publishing-related problems as they arise, is that we would like the embedded |
| 249 | + metadata of the published crates, namely the Git commit at which the release was done, |
| 250 | + to match the Git commit on the `main` branch which will be tagged. |
| 251 | + [See this article][crates.io-security] for a more in-depth explanation. |
| 252 | + **Note:** This step requires the appropriate privileges to push crates to [crates.io]. |
| 253 | +10. Once the tag is pushed, wait for the CI bot to create a GitHub release, |
| 254 | + then update the release description to |
| 255 | + ``` |
| 256 | + [📖 CHANGELOG](https://github.com/cosmos/ibc-proto-rs/blob/master/CHANGELOG.md#vXYZ)` |
| 257 | + ``` |
| 258 | +11. All done! 🎉 |
| 259 | +
|
| 260 | +[crates.io]: https://crates.io |
| 261 | +[crates.io-security]: https://codeandbitters.com/published-crate-analysis/ |
0 commit comments