Skip to content

Commit 5661a2c

Browse files
authoredApr 26, 2023
Update contributing guidelines (informalsystems#3266)
* Add Core Team Responsibility section * Clarify expectations for external PRs * Formatting of bullet point

File tree

1 file changed

+34
-22
lines changed

1 file changed

+34
-22
lines changed
 

‎CONTRIBUTING.md

+34-22
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,38 @@
11
# Contributing
22

3-
Thank you for your interest in contributing! The goal
4-
of ibc-rs is to provide a high quality, formally verified implementation of
5-
the IBC protocol and relayer.
3+
Thank you for your interest in contributing to the Hermes IBC relayer! The goal
4+
of this project is to provide a high quality, formally verified IBC relayer
5+
implementation in Rust.
66

77
All work on the code base should be motivated by a Github
88
issue. Before opening a new issue, first do a search of open and closed issues
99
to make sure that yours will not be a duplicate.
1010
If you would like to work on an issue which already exists, please indicate so
11-
by leaving a comment. If what you'd like to work on hasn't already been covered
12-
by an issue, then open a new one to get the process going.
11+
by leaving a comment on the issue. If what you'd like to work on hasn't already
12+
been covered by an issue, then open a new one to get the process going. Please
13+
do your best to ensure that your issue adheres to one of the
14+
[issue templates](https://github.com/informalsystems/hermes/tree/master/.github/ISSUE_TEMPLATE)
15+
that we have in the repository.
1316

1417
The rest of this document outlines the best practices for contributing to this
1518
repository:
1619

20+
- [Core Team Responsibility](#responsibility) - what the core team is responsible for
1721
- [Decision Making](#decision-making) - process for agreeing to changes
1822
- [Issues](#issues) - what makes a good issue
1923
- [Pull Requests](#pull-requests) - what makes a good pull request
2024
- [Forking](#forking) - fork the repo to make pull requests
2125
- [Changelog](#changelog) - changes must be recorded in the changelog
2226
- [Releases](#releases) - how to release new version of the crates
2327

28+
## Core Team Responsibility
29+
30+
The Hermes core team is responsible for stewarding this project over time. This means that the core team needs to understand the nature of, and agree to maintain, all of the changes that land on the master branch. It may cost a few days or weeks' worth of time to submit a particular change, but maintaining that change over the years has a much higher cost that the core team is responsible for bearing.
31+
32+
With that in mind, the core team must balance the potential risks of maintaining
33+
any contribution in the long-term against the immediate usefulness or utility
34+
that that contribution manifests.
35+
2436
## Decision Making
2537

2638
When contributing to the project, the following process leads to the best chance of
@@ -101,30 +113,31 @@ explaining why, and we will reprioritize it!
101113

102114
## Pull Requests
103115

104-
If you have write access to the ibc-rs repo, you can directly branch off of `master`.
116+
If you have write access to the Hermes repo, you can directly branch off of `master`.
105117
This makes it easier for project maintainers to directly make changes to your
106-
branch should the need arise. Otherwise, check [Forking](#forking) section for instructions.
107-
108-
Branch names should be prefixed with the author's name followed by a short description
109-
of the feature, eg. `name/feature-x`.
118+
branch should the need arise. Otherwise, check the [Forking](#forking) section for instructions.
110119

111-
Pull requests are made against `master` and are squash-merged into master.
112-
113-
PRs must:
120+
Branch names should be prefixed with the author's name followed by a short description of the feature, eg. `name/feature-x`.
114121

122+
Pull requests are made against `master` and are squash-merged into master. Each
123+
PR should:
115124
- make reference to an issue outlining the context
116125
- update any relevant documentation and include tests
117126
- add a corresponding entry in the `.changelog` directory using `unclog`,
118127
see the [Changelog](#changelog) section for more details.
119128

120-
Pull requests should aim to be small and self-contained to facilitate quick
121-
review and merging. Larger change sets should be broken up across multiple PRs.
122-
Commits should be concise but informative, and moderately clean. Commits will be squashed into a
123-
single commit for the PR with all the commit messages.
129+
Additionally, in order to make PRs as easy to review as possible, each PR should:
130+
- Be focused on implementing _*one*_ piece of logic from end-to-end. It must be
131+
very clear what the purpose of the PR is from looking at the PR's title, description, and/or linked issue(s). It should also be very clear what value the changes incorporated in the PR aim to deliver. A single PR that does multiple things, without a clear articulation of the problem it attempts to solve, will very likely be rejected.
132+
- Be small, ideally no more than 500 lines of code changes. While this is a guideline and not a hard rule, in general, larger changes should be structured as a series of PRs, each building off of the previous ones; these PRs should also be tracked in a tracking issue.
133+
134+
If a single PR absolutely has to be larger, it _must_ be structured such that it can be reviewed commit-by-commit, with each commit doing a single logical thing, accompanied with a good description of what it aims to achieve in the git commit message. Poorly structured PRs will likely be rejected on the grounds of being too much of a burden for the core maintainers to review; you will be asked to restructure the PR in accordance with the guidelines laid out here.
135+
136+
This does not necessarily apply to documentation-related changes or automatically generated code (e.g. generated from Protobuf definitions). But automatically generated code changes should occur within separate commits, so they are easily distinguishable from manual code changes.
124137

125138
In order to help facilitate the PR review process, tag *one* person as the
126139
reviewer of the PR. If you are unsure of who to tag, your point of contact on
127-
the ibc-rs team is always a natural candidate; they'll make sure that the PR gets
140+
the Hermes team is always a natural candidate; they'll make sure that the PR gets
128141
reviewed by whomever is most appropriate to review it. It also helps to notify
129142
the person whom you tagged as reviewer through direct means, such as through
130143
Slack or Discord, as it is easy for GitHub notifications to get lost or buried.
@@ -216,7 +229,7 @@ and [Hashicorp Consul](http://github.com/hashicorp/consul/tree/master/CHANGELOG.
216229
See those changelogs for examples.
217230

218231
We currently split changes for a given release between these four sections: Breaking
219-
Changes, Features, Improvements, Bug Fixes.
232+
Changes, Features, Improvements, and Bug Fixes.
220233

221234
Entries in the changelog should initially be logged in the __Unreleased__ section, which
222235
represents a "staging area" for accumulating all the changes throughout a
@@ -272,16 +285,15 @@ Our release process is as follows:
272285
2. All crates' `lib.rs` files documentation references' `html_root_url`
273286
parameters must point to the new version.
274287
3. Every reference to Hermes version in the [guide](./guide).
275-
276288
4. Run `cargo doc --all-features --open` locally to double-check that all the
277-
documentation compiles and seems up-to-date and coherent. Fix any potential
289+
documentation compiles and is up-to-date and coherent. Fix any potential
278290
issues here and push them to the release PR.
279291
5. Mark the PR as **Ready for Review** and incorporate feedback on the release.
280292
6. Once approved, merge the PR.
281293
7. Pull `master` and run the [`release.sh`](./scripts/release.sh) script.
282294
If any problem arises, submit a new PR, get it merged to `master` and try again.
283295
The reason for not releasing straight from the release branch, and therefore losing the
284-
ability to fix publishing problems as they arise, is that we would like the embedded
296+
ability to fix publishing-related problems as they arise, is that we would like the embedded
285297
metadata of the published crates, namely the Git commit at which the release was done,
286298
to match the Git commit on the `master` branch which will be tagged.
287299
[See this article][crates.io-security] for a more in-depth explanation.

0 commit comments

Comments
 (0)
Please sign in to comment.