-
Notifications
You must be signed in to change notification settings - Fork 42
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
Conversation
RELEASE_PROCESS.md
Outdated
|
||
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 comment
The 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 v*
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.
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:
- https://github.com/osmosis-labs/osmosis/blob/main/go.mod#L1
- https://github.com/cosmos/gaia/blob/main/go.mod#L1
and at some point I imagine this becomes a bit of image problem (likeoh you guys have mainnet but you are still on v0.78.0, it sounds sketchy
).
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.
that is true 😅
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.
Do we know why other teams are using the v1
, v2
, etc. method apart from image concerns?
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.
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 v1
, v2
etc is kind of synchronised with upgrades and signal consensus breaking changes. Imo this is pretty nice advantage.
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.
Agreed with all 🎉
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.
Great work!
Maybe it's a good timing to start having PR templates with checklists?
definitly, though I would move to all this things in steps, and imo first step is switching to new process. |
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.
LGTM! Some questions:
CONTRIBUTING.md
Outdated
* 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`. | ||
* Follow branch name conventions: `{prefix}/branch-name`, where `prefix` is one of the | ||
standard [types](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) |
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.
shall we include author name as part of the branch name? e.g., johndoe/feature/add-user-profile
. ref
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.
Yeah, I'd go for this johndoe/feature/add-user-profile
or only johndoe/add-user-profile
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.
tbh for my taste branch names are getting a bit long (super subjective opinion). Are there any other benefits of this approach except of additional information ?
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.
Nope, just easy to track who is working on this branch. We can say this is optional. For someone who wants to add the author name, we recommend going with johndoe/feature/add-user-profile
format
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.
hmm I think being consistent here is more important that any particular format tbh. So how about we copy exactly what Gaia is doing:
- for pr's directly to
main
the name ismoniker/branch-name
, wheremoniker
is github name - for larger pr's we create feature branch with name
feat/feature-branch-name
, and pr's to this branch must follow the namingmoniker/branch-name
CONTRIBUTING.md
Outdated
|
||
* 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 comment
The 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 comment
The 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 feat/**
or */feat/**
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.
How do we enforce this? Can external dev create a feature branch and apply branch protection rules?
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.
CONTRIBUTING.md
Outdated
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. | ||
|
||
Codeowners are marked automatically as the reviewers. |
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.
How do we enforce this?
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.
There is options in github for merging in base branches
We should create CODEOWNERS file to enforce
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.
oh code point about CODEOWNERS, we should definitly introduce that, as one of the first things after the switch
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 comment
The 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 squash and merge
or merge and commit
?
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.
Hmm I think it trunk based approach we can use only squash and merge
as there is no two branches that must have similiar commit like dev
and main
, and release branches are free to diverge. Though I will double check that and possibliy add some info about that
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.
Good to know, if so we should enforce squash and merge
only
RELEASE_PROCESS.md
Outdated
- [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 comment
The reason will be displayed to describe this comment to others. Learn more.
This document outlines the release process for Babylon node (Babylond) | |
This document outlines the release process for Babylon node (babylond) |
In 7448be7 added some improvements based and discussions:
|
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.
Nice work! Added some questions.
I don't see notes on the correct merging method, e.g. commit messages vs. squash and merge. What's the appropriate way?
CONTRIBUTING.md
Outdated
## Overview | ||
|
||
This document codifies rules that must be followed when contributing to | ||
Babylon node repository. |
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.
Babylon node repository. | |
the Babylon node repository. |
CONTRIBUTING.md
Outdated
|
||
## 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Can we wrap up the .md
file into 80-char lines?
CONTRIBUTING.md
Outdated
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 comment
The reason will be displayed to describe this comment to others. Learn more.
The main branch of every Cosmos repository should just build with `go get`, | |
The main branch of every Babylon repository should just build with `go get`, |
RELEASE_PROCESS.md
Outdated
- [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 comment
The reason will be displayed to describe this comment to others. Learn more.
This document outlines the release process for Babylon node (babylond) | |
This document outlines the release process for the Babylon node (babylond). |
RELEASE_PROCESS.md
Outdated
|
||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know why other teams are using the v1
, v2
, etc. method apart from image concerns?
RELEASE_PROCESS.md
Outdated
|
||
* 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 comment
The 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 release/v0.3.x
and release/v0.4.x
(with that being the latest one).
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 comment
The reason will be displayed to describe this comment to others. Learn more.
in general standard process would be to:
- create fix on main
- backport fix to
release/v0.3.x
andrelease/v0.4.x
branches
Now if release/v0.3.x
is old and we already diverged by a lot then:
- pr to main and back port to
release/v0.4.x
- direct pr with fix on
release/v0.3.x
RELEASE_PROCESS.md
Outdated
* 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**. | ||
* 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to create an -rc
branch here?
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.
we do not need to do it, being release candidate becomes a tag concern i.e
- we tag a commit on release branch as
v0.x.y-rc.0
- we test/audit/ do what we want with this commit
- if everything goes well we later tag this commit as
v0.x.y
```bash | ||
git push | ||
``` | ||
### Cutting a new release |
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.
### Cutting a new release | |
### Cutting a new release candidate |
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.
hmm this paragraph is relevent to either rc
on non rc
tag
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.
Ah because below you mention release candidate
highlighted
RELEASE_PROCESS.md
Outdated
``` | ||
### Cutting a new release | ||
|
||
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 comment
The 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 comment
The 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:
we tag a commit on release branch as v0.x.y-rc.0
we test/audit/ do what we want with this commit
if everything goes well we later tag this commit as v0.x.y
@@ -0,0 +1,98 @@ | |||
# Release Process |
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.
Can we please wrap to 80 char lines?
First step of switching to trunk based development process.
Docs are highly influenced by:
But adapted to Babylon situation i.e we do not have yet any testnet, mainnet, nor any stable version released. We also do not have yet:
goreleaser
for building binaries on different platformsAll this things should be added in next steps and documents should be updated to reflect usage of those practices.
After merging this pr, we should merge
dev
tomain
and delete thedev
branch, and follow the process described in those documents.Adding whole team as reviewers, so that we are all on the same page.