Skip to content
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

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

KonradStaniec
Copy link
Collaborator

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:

  • changelog process
  • release note process
  • we do not use goreleaser for building binaries on different platforms

All 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 to main and delete the dev branch, and follow the process described in those documents.

Adding whole team as reviewers, so that we are all on the same page.

CONTRIBUTING.md Outdated Show resolved Hide resolved

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).
Copy link
Contributor

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*

Copy link
Collaborator Author

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:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is true 😅

Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with all 🎉

Copy link
Member

@SebastianElvis SebastianElvis left a 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?

@KonradStaniec
Copy link
Collaborator Author

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.

Copy link
Contributor

@gitferry gitferry left a 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)
Copy link
Contributor

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

Copy link
Member

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

Copy link
Collaborator Author

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 ?

Copy link
Contributor

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

Copy link
Collaborator Author

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:

  1. for pr's directly to main the name is moniker/branch-name, where moniker is github name
  2. for larger pr's we create feature branch with name feat/feature-branch-name , and pr's to this branch must follow the naming moniker/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.
Copy link
Contributor

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?

Copy link
Contributor

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/**

Copy link
Collaborator Author

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.
Copy link
Contributor

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?

Copy link
Contributor

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
image

We should create CODEOWNERS file to enforce

Copy link
Collaborator Author

@KonradStaniec KonradStaniec Aug 21, 2024

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
Copy link
Contributor

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?

Copy link
Collaborator Author

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

Copy link
Contributor

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

- [Tagging Procedure](#tagging-procedure)
- [Patch release Procedure](#patch-release-procedure)

This document outlines the release process for Babylon node (Babylond)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This document outlines the release process for Babylon node (Babylond)
This document outlines the release process for Babylon node (babylond)

@KonradStaniec
Copy link
Collaborator Author

In 7448be7 added some improvements based and discussions:

  • branches should be named {moniker}/branch-name
  • for now removed mentions of CODEOWNERS and checklists, as we still do not have those so the documents should be updated when we add them

Copy link
Member

@vitsalis vitsalis left a 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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`.
Copy link
Member

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`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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`,

- [Tagging Procedure](#tagging-procedure)
- [Patch release Procedure](#patch-release-procedure)

This document outlines the release process for Babylon node (babylond)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This document outlines the release process for Babylon node (babylond)
This document outlines the release process for the Babylon node (babylond).


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).
Copy link
Member

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?


* 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**.
Copy link
Member

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?

Copy link
Collaborator Author

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 and release/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

* 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.
Copy link
Member

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?

Copy link
Collaborator Author

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Cutting a new release
### Cutting a new release candidate

Copy link
Collaborator Author

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

Copy link
Member

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

```
### Cutting a new release

Before cutting a _**release candidate**_ (e.g., `v0.10.0-rc0`), the following steps are necessary:
Copy link
Member

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?

Copy link
Collaborator Author

@KonradStaniec KonradStaniec Aug 26, 2024

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
Copy link
Member

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?

@KonradStaniec KonradStaniec merged commit f45d16f into dev Aug 26, 2024
17 checks passed
@KonradStaniec KonradStaniec deleted the doc/release-contribution-docs branch August 26, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants