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 ReleaseProcess.md #4207

Merged
merged 31 commits into from
Jan 23, 2023
Merged

Add ReleaseProcess.md #4207

merged 31 commits into from
Jan 23, 2023

Conversation

nfrisby
Copy link
Contributor

@nfrisby nfrisby commented Dec 1, 2022

Closes #4204

I wrote the ReleaseProcessDecision.md to capture and edit the notes from the various couple sync calls where we discussed how to version our packages.

Those discussions expanded a bit into how to maintain/prepare releases. That results in the ReleaseProcess.md file, which documents our process---it's not just a record of/catalyst for discussion like ReleaseProcessDecision.md was.


- *Simplicity and Familiarity*. We'd like the versioning scheme to be simple to explain, and ideally already well-established.

- *Ease of execution*. We'd like the process of cutting a release to merely involve following a very simple checklist. We'd like it to require as few inputs, discussions, decisions, etc.
Copy link
Contributor Author

@nfrisby nfrisby Dec 1, 2022

Choose a reason for hiding this comment

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

TODO The main thing we've discussed that is missing from the document is deciding that some packages will advance in lock-step. We had considered this partitioning:

ouroboros-consensus.cabal
ouroboros-consensus-diffusion.cabal
ouroboros-consensus-test.cabal
ouroboros-consensus-mock.cabal
ouroboros-consensus-mock-test.cabal
ouroboros-consensus-protocol.cabal

ouroboros-consensus-byron.cabal
ouroboros-consensus-byron-test.cabal
ouroboros-consensus-byronspec.cabal
ouroboros-consensus-cardano.cabal
ouroboros-consensus-cardano-test.cabal
ouroboros-consensus-cardano-tools.cabal
ouroboros-consensus-shelley.cabal
ouroboros-consensus-shelley-test.cabal

Master always has degenerate versions on it: everything is version 0.
To cut a release, create branch release/XXX pointing at the desired master commit and then add a commit to release/XXX that advances all appropriate versions COMPARED TO their value in the previous release branch.

## Proposal Dimension
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I currently favor Proposal Dimension, FWIW.

Copy link
Contributor

Choose a reason for hiding this comment

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

I favor this one too! It also allows adding patch on release (hotfix).

Copy link
Member

Choose a reason for hiding this comment

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

It also allows adding patch on release (hotfix).

I think whether we are able to create patch releases is mostly orthogonal to the different proposals here -- i.e. Proposal Dimension and Proposal NonZero only differ in what version is written down in the .cabal files on master, but the versions in CHaP would/could be exactly the same.

Copy link
Contributor Author

@nfrisby nfrisby Dec 1, 2022

Choose a reason for hiding this comment

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

We can have a call to get into the comparisons (with an eventually corresponding section in this document), but: I prefer Dimension to NonZero because it respects two additional desiderata:

  • it is always the case that a build that is more refined will have a greater version (cf Distinguished development versions, which includes "..., which is always somehow more refined than any release that has a lesser version number")

  • when bumping the versions for the release branch, you only need the master branch, you don't have to also lookup the versions from the previous release branch. (cf Ease of execution, which includes "We'd like it to require as few inputs, ...")

I think we had discussed that first bullet last time, and you had explained that there are relatively easy ways to mitigate it, but I personally prefer just maintaining the invariant (which I find broadly intuitive).

Copy link
Contributor

Choose a reason for hiding this comment

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

I also like this one the most, it's quite clean. It has the nice side effect that all your packages will have the same major version if I understand correctly. This makes it quite nice for downstream dependencies. Further more if you will have some reasonable constraints between all your packages, then declaration of which version of consensus is used might be as simple as constraining only one of consensus packages in cardano-node repo.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also ok with this proposal.

Copy link
Member

Choose a reason for hiding this comment

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

However, I like this option better:

Whenever a change is done on master (or a feature branch before), consider how it impacts the version number, i.e. bump the corresponding digit for breaking changes or patches when you do break an interface or just patch something. The master version then indicates the version number which would be true if you just release today (e.g. by tagging a commit).

@@ -0,0 +1,34 @@
# Choosing a Versioning Schemes

This document records our discussions around choosing how to version our packages.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once we've made a decision inside Consensus, we can invite Marcin et al to ask whether they agree/can refine?

@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch from 31e3bc4 to fa4a9fc Compare December 1, 2022 18:10
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Why create release branches? Why not simply tag the release from master? Do you foresee a situation where you would need to fix specific versions and not others? Consensus is a library which currently is only used in a single place (cardano-node).
For the quickcheck-dynamic library, we've setup a simple scheme where versions are just tags on master and pushed to hackage from CI: https://github.com/input-output-hk/quickcheck-dynamic/blob/main/.github/workflows/release.yaml
Perhaps a simple scheme like that could work for consensus?
Also, are you using SemVer or PVP?


## Proposal Simplest

To cut a release, merely create branch release/XXX pointing at the desired commit on master and also immediately create a subsequent master commit that advances all the appropriate versions.
Copy link
Member

Choose a reason for hiding this comment

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

create branch release/XXX

Just to be sure: In all proposals listed here, cutting a release involves "create branch release/XXX", i.e. you would create a new branch for every version -- is this intentional?

E.g. right now, we just have one release branch (release/cardano-node-1.35.x), which was forked off of master, and only contains backported commits and version bumps, which are referenced in CHaP.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest using tags, instead of branches for releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also in favour of tags, but branches might be needed at times so I wouldn't rule them out. This is because the networking team might push changes to master which we might not want to release in a hot fix release, but rather a major release; or the other way around: like currently with 1.35.5 where I needed to push >100 commits to the release branch to avoid releasing consensus changes. That's actually another argument to split network & consensus into two repos (I am starting to convince myself that's actually a good thing to do).

If we had two repositories, we would integrate network changes to master, but then to try it out, we would likely cherry pick changes on the most recent ouroboros-consensus released with cardano-node. Since now only ouroboros-consensus-diffusion depends on ouroboros-network, merging our changes to master might be enough to avoid using not yet released ouroboros-consensus with our new stuff. If we add a new version of NodeToNodeVersion (or NodeToClientVersion) then we will also need a newer version of ouroboros-consensus-cardano for which we might need to use a release branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is because the networking team might push changes to master which we might not want to release in a hot fix release, but rather a major release; or the other way around:

In such situation you can always create a branch from a tag, work on it and once such fix has finalized and release was made it should be turned into another tag. Branches are intended for mutable things, while tags for immutable, such as releases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to release all the consensus packages at the same time, or only release packages which have some changes? Probably the former, but it's better to be explicit about it.

Copy link
Contributor Author

@nfrisby nfrisby Dec 1, 2022

Choose a reason for hiding this comment

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

Sorry, yes, I wrote too loosely. I should have consistently said:

To cut a release of a branch brand new minor version, create a branch release/foo-A.B.x and ...

I think that's our plan (and we'll tag increments within that branch with concrete versions release/foo-A.B.3).

Edit: fixed the typo Alexey caught below

Copy link
Contributor

Choose a reason for hiding this comment

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

🙂 perhaps release/foo-A.B.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cleaned it up to just say "announce a commit" instead of talking about branches, tags, etc. Thanks!

@lehins
Copy link
Contributor

lehins commented Dec 1, 2022

I always assumed versioning scheme should be decided at the CHaP's level. This way all packages in IO could conform to it and there would be no surprises or inconsistencies. I was a strong proponent of PVP for Haskell packages, as the rest of the Haskell community relies on it.

Some things discussed in this document go against common versioning schemes like PVP and SemVer. For example if you release all packages in the repo at once and they all have the same version, then you are back to the same scheme as it was prior to CHaPs using one version for the whole repo (previously it was git sha), except with a more convoluted mechanism.

@nfrisby
Copy link
Contributor Author

nfrisby commented Dec 1, 2022

Both #4207 (review) and #4207 (comment) seem out of scope to me.

That's probably my fault, for using shorthand (approximately synecdoche) in the write-up.

To clarify, this PR is focused on one question: for each individual package, how should the version listed in the .cabal file on the master branch relate to the version in the .cabal file in the latest release?

In particular, I think this question is almost entirely independent of branches versus tags, SemVer versus PVP, etc.

Specific conflicts:

  • I think SemVer requires x.y.z, and Proposal Dimensions would only have x.y on the master branch, but every release would be x.y.z. So that seems 99% compliant to me.

  • SemVer and PVP, as I recall, at least primarily focus on when to increment which dimension of the version. However, I don't think it would be a earth-shattering violation to increase the version by more than one (cf Proposal Parity), or to increase it more than strictly necessary (cf the GitHub comment TODO about keeping subsets of our packages in lock-step).

Thanks for the feedback! I am happy to have it, just not necessarily on this PR :D (again: my fault).

@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch 3 times, most recently from 82bf4ba to 7f97ceb Compare December 1, 2022 21:42
@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch from 7f97ceb to 10ea46a Compare December 1, 2022 21:43
Copy link

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

Not sure if any of your proposals already covers this approach: Whenever a change is done on master (or a feature branch before), consider how it impacts the version number, i.e. bump the corresponding digit for breaking changes or patches when you do break an interface or just patch something. The master version then indicates the version number which would be true if you just release today (e.g. by tagging a commit). We use that also in hydra

Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

I think you're missing an option, let's call it "Proposal Retrograde": when you want to make a release, check all the changes that have been made since the last release to decide whether a minor/major version bump is needed, perform it, and then create the release branch.

That and "Simplest" are the two I've seen most often, basically: either bump your versions just after releasing or just before releasing. The advantage of doing it just after is that it's easy to remember, and your master version is always on a newer version; the advantage of doing it just before is that it's easier to compute the version bump that is required, if you're doing major/minor/patch versions specially (hard to do in advance!).

Parity also has the forward-looking problem: GHC essentially commits that the next release off master will always be a second-major version, and all minor/patch releases happen off a release branch. That means that they can correctly guess the "next" version ahead of time. That may or may not be true for you.

## Desiderata

- *Conventional Version Number*.
We'd like our release versions to have the standard shape of `A.B.C`, where in particular increments of `C` principally represent bugfixes.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this does bear on PVP, since PVP has two major versions so C would be a minor version rather than a patch version.

Copy link
Contributor Author

@nfrisby nfrisby Dec 2, 2022

Choose a reason for hiding this comment

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

For the record, I think 2 3 dimensions versus 3 4 dimensions is a superficial difference. The differences discussed at https://pvp.haskell.org/faq/#how-does-the-pvp-relate-to-semantic-versioning-semver all seem secondary to me.

Copy link
Contributor Author

@nfrisby nfrisby Dec 2, 2022

Choose a reason for hiding this comment

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

I've addressed this somewhat-directly in the Desiderata section now.

@michaelpj
Copy link
Contributor

Whenever a change is done on master (or a feature branch before), consider how it impacts the version number, i.e. bump the corresponding digit for breaking changes or patches when you do break an interface or just patch something.

The downside of this is that if you do two changes that require a minor bump, then you might accidentally bump the minor version twice. You could fix that up before you do a release, but then I'm not sure it's better than calculating the version bump just before you release.

@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch 6 times, most recently from c4dbc30 to eb79566 Compare January 12, 2023 19:35
@nfrisby nfrisby changed the title Add VersioningSchemeDecision.md Add ReleaseProcess.md Jan 12, 2023
@nfrisby nfrisby added the consensus issues related to ouroboros-consensus label Jan 12, 2023
@nfrisby nfrisby marked this pull request as ready for review January 12, 2023 19:38
@nfrisby
Copy link
Contributor Author

nfrisby commented Jan 12, 2023

I've marked this PR ready for review. The main content is the new ReleaseProcess.md file; the other one is just for historical reference if we end up later questioning the content of ReleaseProcess.md.

@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch 4 times, most recently from db544d1 to fea8072 Compare January 12, 2023 19:48
@nfrisby nfrisby force-pushed the nfrisby/versioning-scheme branch from fea8072 to 6da5831 Compare January 12, 2023 19:52
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
Copy link
Member

@dnadales dnadales left a comment

Choose a reason for hiding this comment

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

Thank you Nick! I left some comments and questions.

ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
@nfrisby nfrisby mentioned this pull request Jan 18, 2023
9 tasks
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

Thanks, this is a very well-structured and readable overview!

ouroboros-consensus/docs/ReleaseProcess.md Show resolved Hide resolved
ouroboros-consensus/docs/ReleaseProcess.md Outdated Show resolved Hide resolved
@nfrisby
Copy link
Contributor Author

nfrisby commented Jan 23, 2023

bors merge

@iohk-bors iohk-bors bot merged commit 8def57b into master Jan 23, 2023
@iohk-bors iohk-bors bot deleted the nfrisby/versioning-scheme branch January 23, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decide on a versioning scheme for the Consensus packages