-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add release process and contributing docs #27
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,143 @@ | ||||||
# Contributing | ||||||
|
||||||
- [Contributing](#contributing) | ||||||
- [Overview](#overview) | ||||||
- [Pull request rules](#pull-request-rules) | ||||||
- [Development Procedure](#development-procedure) | ||||||
- [Testing](#testing) | ||||||
- [Pull Requests](#pull-requests) | ||||||
- [Requesting Reviews](#requesting-reviews) | ||||||
- [Updating Documentation](#updating-documentation) | ||||||
- [Dependencies](#dependencies) | ||||||
- [Protobuf](#protobuf) | ||||||
- [Branching Model and Release](#branching-model-and-release) | ||||||
- [PR Targeting](#pr-targeting) | ||||||
|
||||||
## Overview | ||||||
|
||||||
This document codifies rules that must be followed when contributing to | ||||||
Babylon node repository. | ||||||
|
||||||
### Pull request rules | ||||||
|
||||||
Every pull request must be easy to review. To make it possible: | ||||||
|
||||||
1. **Each pull request must do _one thing_**. It must be very clear what that | ||||||
one thing is when looking at the pull request title, description, and linked | ||||||
issues. It must also be very clear what value it ultimately aims to deliver, | ||||||
and for which user(s). | ||||||
|
||||||
2. **Each pull request must be manageable in size**. | ||||||
Self-contained pull requests that are manageable in size may target `main` directly. | ||||||
Larger contributions though must be structured as a series of smaller pull requests | ||||||
each building upon the previous one, all ideally tracked in a tracking issue | ||||||
These pull requests must target a long-lived feature branch. | ||||||
For details, see the [development procedure guidelines](#development-procedure). | ||||||
|
||||||
**Note**: 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. | ||||||
|
||||||
## Development Procedure | ||||||
|
||||||
`main` must be stable, include only completed features and never fail `make test`, `make test-e2e`, or `make build/install`. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we wrap up the |
||||||
|
||||||
Depending on the scope of the work, we differentiate between self-contained pull requests and long-lived contributions (features). | ||||||
|
||||||
**Self-contained pull requests**: | ||||||
|
||||||
* Fork the repo (core developers must create a branch directly in the Babylon repo), | ||||||
branch from the HEAD of `main`, make some commits, and submit a PR to `main`. | ||||||
* For developers who are core contributors and are working within the Babylon repo, follow branch name conventions to ensure clear | ||||||
ownership of branches: `{moniker}/branch-name`. | ||||||
* See [Branching Model](#branching-model-and-release) for more details. | ||||||
|
||||||
**Large contributions**: | ||||||
|
||||||
* Make sure that a feature branch is created in the repo or create one. | ||||||
The name convention for the feature branch must be `feat/branch-name`. | ||||||
Note that (similar to `main`) all feature branches have branch protection rules and they run the CI. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we enforce this? Can external dev create a feature branch and apply branch protection rules? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can set patterns in the workflows file to run in pattern branches There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Hmm general flow seems that feature branches are usually reserved for bigger features, so for external contributor to go for such bigger feature it should be first discussed with core team, and core team can create such feature branch. (that is at least what Gaia does) In general, I kind of treated external contributors with less details for now as imo more important is to have core team to agree on the new flow. |
||||||
Unlike `main`, feature branch may intermittently fail `make test`, `make test-e2e`, or `make build/install`. | ||||||
* Fork the repo (core developers must create a branch directly in the Babylon repo), | ||||||
branch from the HEAD of the feature branch, make some commits, and submit a PR to the feature branch. | ||||||
All PRs targeting a feature branch should follow the same guidelines in this document. | ||||||
* Once the feature is completed, submit a PR from the feature branch targeting `main`. | ||||||
|
||||||
### Testing | ||||||
|
||||||
Tests can be executed by running `make test` at the top level of the Babylon repository. | ||||||
Running e2e test can be accomplished by running `make test-e2e` | ||||||
|
||||||
### Pull Requests | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to add merge rules? For example, when can we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I think it trunk based approach we can use only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good to know, if so we should enforce |
||||||
|
||||||
Before submitting a pull request: | ||||||
|
||||||
* synchronize your branch with the latest base branch (i.e., `main` or feature branch) and resolve any arising conflicts, e.g., | ||||||
- either `git fetch origin/main && git merge origin/main` | ||||||
- or `git fetch origin/main && git rebase -i origin/main` | ||||||
* run `make test`, `make test-e2e`, `make build/install` to ensure that all checks and tests pass. | ||||||
|
||||||
Then: | ||||||
|
||||||
1. If you have something to show, **start with a `Draft` PR**. It's good to have early validation of your work and we highly recommend this practice. A Draft PR also indicates to the community that the work is in progress. | ||||||
2. When the code is complete, change your PR from `Draft` to `Ready for Review`. | ||||||
|
||||||
PRs must have a category prefix that is based on the type of changes being made (for example, `fix`, `feat`, | ||||||
`refactor`, `docs`, and so on). The [type](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) | ||||||
must be included in the PR title as a prefix (for example, `fix: <description>`). | ||||||
This convention ensures that all changes that are committed to the base branch follow the | ||||||
[Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification. | ||||||
|
||||||
### Requesting Reviews | ||||||
|
||||||
If you would like to receive early feedback on the PR, open the PR as a "Draft" and leave a comment in the PR indicating | ||||||
that you would like early feedback and tagging whoever you would like to receive feedback from. | ||||||
|
||||||
All PRs require at least two review approvals before they can be merged (one review might be acceptable in | ||||||
the case of minor changes or changes that do not affect production code). | ||||||
|
||||||
### Updating Documentation | ||||||
|
||||||
If you open a PR in Babylon, it is mandatory to update the relevant documentation in `/docs`. | ||||||
|
||||||
## Dependencies | ||||||
|
||||||
We use [Go Modules](https://github.com/golang/go/wiki/Modules) to manage | ||||||
dependency versions. | ||||||
|
||||||
The main branch of every Cosmos repository should just build with `go get`, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
which means they should be kept up-to-date with their dependencies so we can | ||||||
get away with telling people they can just `go get` our software. | ||||||
|
||||||
When dependencies in Babylon `go.mod` are changed, it is generally accepted practice | ||||||
to delete `go.sum` and then run `go mod tidy`. | ||||||
|
||||||
Since some dependencies are not under our control, a third party may break our | ||||||
build, in which case we can fall back on `go mod tidy -v`. | ||||||
|
||||||
## Protobuf | ||||||
|
||||||
We use [Protocol Buffers](https://developers.google.com/protocol-buffers) along with [gogoproto](https://github.com/cosmos/gogoproto) to generate code for use in Babylon. | ||||||
|
||||||
For deterministic behavior around Protobuf tooling, everything is containerized using Docker. Make sure to have Docker installed on your machine, or head to [Docker's website](https://docs.docker.com/get-docker/) to install it. | ||||||
|
||||||
To generate the protobuf stubs, you can run `make proto-gen`. | ||||||
|
||||||
## Branching Model and Release | ||||||
|
||||||
User-facing repos should adhere to the trunk based development branching model: https://trunkbaseddevelopment.com. User branches should start with a user name, example: `{moniker}/branch-name`. | ||||||
|
||||||
Babylon follows [semantic versioning](https://semver.org), but with the some deviations to account for state-machine and API breaking changes. See [RELEASE_PROCESS.md](./RELEASE_PROCESS.md) for details. | ||||||
|
||||||
### PR Targeting | ||||||
|
||||||
Ensure that you base and target your PRs on either `main` or a feature branch. | ||||||
|
||||||
All complete features and bug fixes must be targeted against `main`. | ||||||
|
||||||
Exception is for bug fixes which are only related to a released version. | ||||||
In that case: | ||||||
- either, bug fix must be targeted at `main` branch and later back ported to `release/` branch | ||||||
- or if `main` and `release/` branched diverged too much, the fix can be targeted | ||||||
to `release/` branch directly |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,98 @@ | ||||||
# Release Process | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we please wrap to 80 char lines? |
||||||
|
||||||
- [Release Process](#release-process) | ||||||
- [Breaking Changes](#breaking-changes) | ||||||
- [Release Procedure](#release-procedure) | ||||||
- [Creating a new release branch](#creating-a-new-release-branch) | ||||||
- [Cutting a new release](#cutting-a-new-release) | ||||||
- [Tagging Procedure](#tagging-procedure) | ||||||
- [Patch release Procedure](#patch-release-procedure) | ||||||
|
||||||
This document outlines the release process for Babylon node (babylond) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
Babylon follows [semantic versioning](https://semver.org), but with the following deviations to account for state-machine and API breaking changes. | ||||||
|
||||||
- State-machine breaking changes & API breaking changes will result in an increase of the minor version Y (0.Y.z). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is nice for us to keep using v0... that helps us to avoid updating dependencies / go mods with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so too, though we will probably need to release v1 at some point in time, as most chains are doing this that way:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that is true 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we know why other teams are using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not ask anybody tbh so it is hard to tell exact specifics reasons. Another reason I can things of though is that specifically Gaia, uses major version to signal state breaking change, and minor version to signal api breaking one. So each |
||||||
- All other changes will result in an increase of the patch version Z (0.y.Z). | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So patches are not anymore only for hotfixes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, if we have minor non state breaking, non api breaking change we can release it as patch version |
||||||
|
||||||
## Breaking Changes | ||||||
|
||||||
A change is considered to be ***state-machine breaking*** if it requires a coordinated upgrade for the network to preserve state compatibility | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about hotfixes that involve state concerns? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so here we are bit constrained by the fact we do not have v1. In our case, I would say we need to either:
I think I would lean to hot-fix which breaks state in such ciriticial situation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup I revised text in
as until we have After we release |
||||||
Note that when bumping the dependencies of [Cosmos SDK](https://github.com/cosmos/cosmos-sdk) and [IBC](https://github.com/cosmos/ibc-go), | ||||||
we will only treat patch releases as non state-machine breaking. | ||||||
|
||||||
A change is considered to be ***API breaking*** if it modifies the provided API. This includes events, queries, CLI interfaces. | ||||||
|
||||||
## Release Procedure | ||||||
|
||||||
A _release_ is an increment of the second number (eg: `v0.1.0` → `v0.2.0`) | ||||||
|
||||||
**Note**: Generally, PRs should target either `main` or a long-lived feature branch (see [CONTRIBUTING.md](./CONTRIBUTING.md#pull-requests)). | ||||||
|
||||||
* Once the team feels that `main` is _**feature complete**_, we create a `release/v0.Y.x` branch (going forward known as release branch), | ||||||
where `Y` is the minor version number, with patch part substituted to `x` (eg: v0.11.x). | ||||||
* **PRs targeting directly a release branch can be merged _only_ when exceptional circumstances arise**. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens on those exceptional situations? Let's say we find a bug that affects We need to release new patches for v0.3.x and v0.4.x. How do we accomplish that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in general standard process would be to:
Now if
|
||||||
* We freeze the release branch from receiving any new features and focus on releasing a release candidate. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to create an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we do not need to do it, being release candidate becomes a tag concern i.e
|
||||||
* Finish audits and reviews. | ||||||
* Add more tests. | ||||||
* Fix bugs as they are discovered. | ||||||
* After the team feels that the release branch works fine, we cut a release candidate. | ||||||
* Create a new annotated git tag for a release candidate in the release branch (follow the [Tagging Procedure](#tagging-procedure)). | ||||||
* The release verification on devnet must pass. | ||||||
* When bugs are found, create a PR for `main`, and backport fixes to the release branch. | ||||||
* Create new release candidate tags after bugs are fixed. | ||||||
* After the team feels the release candidate is ready, create a full release: | ||||||
* **Note:** The final release MUST have the same commit hash as the latest corresponding release candidate. | ||||||
* Create a new annotated git tag in the release branch (follow the [Tagging Procedure](#tagging-procedure)) | ||||||
|
||||||
### Creating a new release branch | ||||||
|
||||||
- create a new release branch, e.g., `release/v0.10.x` | ||||||
```bash | ||||||
git checkout main | ||||||
git pull | ||||||
git checkout -b release/v0.10.x | ||||||
``` | ||||||
- push the release branch upstream | ||||||
```bash | ||||||
git push | ||||||
``` | ||||||
### Cutting a new release | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm this paragraph is relevent to either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah because below you mention |
||||||
|
||||||
Before cutting a _**release candidate**_ (e.g., `v0.10.0-rc0`), the following steps are necessary: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about a normal release? When does a release candidate become a release? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the answer is a bit vague, when we feel confident about it. As described in other comments:
|
||||||
|
||||||
- move to the release branch, e.g., `release/v0.10.x` | ||||||
```bash | ||||||
git checkout release/v0.10.x | ||||||
``` | ||||||
- create new tag (follow the [Tagging Procedure](#tagging-procedure)) | ||||||
|
||||||
### Tagging Procedure | ||||||
|
||||||
**Important**: _**Always create tags from your local machine**_ since all release tags should be signed and annotated. | ||||||
Using Github UI will create a `lightweight` tag, so it's possible that `babylond version` returns a commit hash, instead of a tag. | ||||||
This is important because most operators build from source, and having incorrect information when you run `make install && babylond version` raises confusion. | ||||||
|
||||||
The following steps are the default for tagging a specific branch commit using git on your local machine. Usually, release branches are labeled `release/v*`: | ||||||
|
||||||
Ensure you have checked out the commit you wish to tag and then do (assuming you want | ||||||
to release version `v0.10.0-rc0` ): | ||||||
```bash | ||||||
git pull --tags | ||||||
|
||||||
git tag -s -a v0.10.0-rc0 -m "Version v0.10.0-rc0" | ||||||
``` | ||||||
## Patch Release Procedure | ||||||
|
||||||
A _patch release_ is an increment of the patch number (eg: `v10.0.0` → `v10.0.1`). | ||||||
|
||||||
**Important**: _**Patch releases must not break consensus.**_ | ||||||
|
||||||
Updates to the release branch should come from `main` by backporting PRs | ||||||
(usually done by automatic cherry pick followed by a PRs to the release branch). | ||||||
The backports must be marked using `backport/Y` label in PR for main. | ||||||
It is the PR author's responsibility to fix merge conflicts and | ||||||
ensure CI passes. | ||||||
|
||||||
After the release branch has all commits required for the next patch release: | ||||||
* Create a new annotated git tag in the release branch (follow the [Tagging Procedure](#tagging-procedure)). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.