From 3922189aff6cb59d2582aa5c2915724276d8d106 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Tue, 22 Aug 2023 08:06:09 -0700 Subject: [PATCH 1/5] Create adr-011-separate-releasing.md --- docs/docs/adrs/adr-011-separate-releasing.md | 72 ++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 docs/docs/adrs/adr-011-separate-releasing.md diff --git a/docs/docs/adrs/adr-011-separate-releasing.md b/docs/docs/adrs/adr-011-separate-releasing.md new file mode 100644 index 0000000000..8dcb3d1e55 --- /dev/null +++ b/docs/docs/adrs/adr-011-separate-releasing.md @@ -0,0 +1,72 @@ +--- +sidebar_position: 12 +title: Separate Releasing +--- +# ADR 011: Separate Releasing + +## Changelog + +* {8/18/22}: Initial draft of idea in [#801](https://github.com/cosmos/interchain-security/issues/801) +* {8/22/22}: Put idea in this ADR + +## Status + +Accepted + +## Context + +### Spike results + +I explored the idea of [#801](https://github.com/cosmos/interchain-security/issues/801) with this [spike branch](https://github.com/cosmos/interchain-security/tree/shawn%2Fgo-mod-split-aug-spike). Here's my conclusions: + +Splitting this repo to have multiple go.mods is possible. However there are various intricacies involved in decoupling the package hierarchy to have `x/ccv/types` as the lowest level dep, with `x/ccv/consumer` and `x/ccv/provider` being one dep layer above, with high-level tests depending on all three of the mentioned packages. I'd estimate this decoupling would take 2-5 workdays to finish, and require significant review effort. + +### Why go.mod split is not the way to go + +Let's take a step back and remember the issue we're trying to solve - **We need a clean way to decouple semver/releasing for the consumer and provider modules**. After more consideration, splitting up go.mods gives us little benefit in achieving this. Reasons: + +* The `go.mod` dependency system is tied to git tags for the entire repo (ex: `require github.com/cometbft/cometbft v0.37.2` refers to a historical tag for the entire cometbft repo). +* It'd be an odd dev experience to allow modules to reference past releases of other modules in the same repo. When would we ever want the consumer module to reference a past release of the types module for example? +* If we allow for `go.mod` replace statements to build from local source code, why split up the package deps at all? +* Splitting go.mods adds a bunch of complexity with `go.work` files and all that shiz. VSCode does not play well with multiple module repos either. + +### Why separate repos is cool but also not the way to go + +All this considered, the cleanest solution to decoupling semver/releasing for the consumer and provider modules would be to have multiple repos, each with their own go.mod (3-4 repos total including high level tests). With this scheme we could separately tag each repo as changes are merged, they could share some code from `types` being an external dep, etc. + +I don't think any of us want to split up the monorepo, that's a lot of work and seems like bikeshedding. There's another solution that's very simple.. + +## Decision + +Slightly adapting [the current semver ruleset](https://github.com/cosmos/interchain-security/blob/cca008d856e3ffc60ec1a486871d0faa702abe26/CONTRIBUTING.md#semantic-versioning): + +* A library API breaking change TO EITHER MODULE will result in an increase of the MAJOR version number for both the consumer and provider modules. Most often this will involve bumping a major version of sdk or a large change to the protocol that warrants new a major version for consumer/provider anyways. +* A state breaking change (change requiring coordinated upgrade and/or state migration) will result in an increase of the MINOR version number FOR THAT MODULE (consumer-x.Y.z or provider-x.Y.z ). +* Any other changes (including node API breaking changes) will result in an increase of the PATCH version number FOR THAT MODULE (consumer-x.y.Z or provider-x.y.Z). + +For each PR merged to main (something like this should be in our PR template): + +* If this PR is library API breaking, bump the go.mod version string of the repo, and the PR will result in a new major release for the consumer and provider. +* Else if this PR is state breaking to consumer, provider, or both, the PR will accordingly result in new minor release(s). +* Else if this PR has other changes, the PR will accordingly result in new patch release(s). + +## Consequences + +### Positive + +* Consumer repos have clear communication of what tagged versions are relevant to them. Consumer devs should know to never reference an ICS version that starts with `provider`, even if it'd technically build. +* Consumer and provider modules do not deviate as long as we continually release off a shared main branch. Backporting remains relatively unchanged besides being explicit about what module(s) your changes should affect. +* No code changes, just changes in process. Very simple. + +### Negative + +* Slightly more complexity. + +### Neutral + +## References + +> Are there any relevant PR comments, issues that led up to this, or articles referenced for why we made the given design choice? If so link them here! + +* [#801](https://github.com/cosmos/interchain-security/issues/801) +* [#801 comment](https://github.com/cosmos/interchain-security/issues/801#issuecomment-1683349298) From 2732fe2f9f6354f91ae6416b078080bdef4278bb Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 23 Aug 2023 10:08:14 -0700 Subject: [PATCH 2/5] Update docs/docs/adrs/adr-011-separate-releasing.md Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> --- docs/docs/adrs/adr-011-separate-releasing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/adrs/adr-011-separate-releasing.md b/docs/docs/adrs/adr-011-separate-releasing.md index 8dcb3d1e55..e710e2d114 100644 --- a/docs/docs/adrs/adr-011-separate-releasing.md +++ b/docs/docs/adrs/adr-011-separate-releasing.md @@ -61,7 +61,7 @@ For each PR merged to main (something like this should be in our PR template): ### Negative * Slightly more complexity. - +* This solution does not allow having provider and consumer on separate versions of e.g. the Cosmos SDK ### Neutral ## References From 4447592f64bea87af50686e0e53cb436e38022f2 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 23 Aug 2023 10:10:26 -0700 Subject: [PATCH 3/5] adr 12 not 11 --- ...11-separate-releasing.md => adr-012-separate-releasing.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename docs/docs/adrs/{adr-011-separate-releasing.md => adr-012-separate-releasing.md} (98%) diff --git a/docs/docs/adrs/adr-011-separate-releasing.md b/docs/docs/adrs/adr-012-separate-releasing.md similarity index 98% rename from docs/docs/adrs/adr-011-separate-releasing.md rename to docs/docs/adrs/adr-012-separate-releasing.md index e710e2d114..f2d42767b0 100644 --- a/docs/docs/adrs/adr-011-separate-releasing.md +++ b/docs/docs/adrs/adr-012-separate-releasing.md @@ -1,8 +1,8 @@ --- -sidebar_position: 12 +sidebar_position: 13 title: Separate Releasing --- -# ADR 011: Separate Releasing +# ADR 012: Separate Releasing ## Changelog From 39dbb17aca85ba3cc666ea93d8e0fba6d58144f2 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 23 Aug 2023 10:13:40 -0700 Subject: [PATCH 4/5] correct that we use postfix not prefix --- docs/docs/adrs/adr-012-separate-releasing.md | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/docs/docs/adrs/adr-012-separate-releasing.md b/docs/docs/adrs/adr-012-separate-releasing.md index f2d42767b0..e0a4a8a470 100644 --- a/docs/docs/adrs/adr-012-separate-releasing.md +++ b/docs/docs/adrs/adr-012-separate-releasing.md @@ -40,15 +40,9 @@ I don't think any of us want to split up the monorepo, that's a lot of work and Slightly adapting [the current semver ruleset](https://github.com/cosmos/interchain-security/blob/cca008d856e3ffc60ec1a486871d0faa702abe26/CONTRIBUTING.md#semantic-versioning): -* A library API breaking change TO EITHER MODULE will result in an increase of the MAJOR version number for both the consumer and provider modules. Most often this will involve bumping a major version of sdk or a large change to the protocol that warrants new a major version for consumer/provider anyways. -* A state breaking change (change requiring coordinated upgrade and/or state migration) will result in an increase of the MINOR version number FOR THAT MODULE (consumer-x.Y.z or provider-x.Y.z ). -* Any other changes (including node API breaking changes) will result in an increase of the PATCH version number FOR THAT MODULE (consumer-x.y.Z or provider-x.y.Z). - -For each PR merged to main (something like this should be in our PR template): - -* If this PR is library API breaking, bump the go.mod version string of the repo, and the PR will result in a new major release for the consumer and provider. -* Else if this PR is state breaking to consumer, provider, or both, the PR will accordingly result in new minor release(s). -* Else if this PR has other changes, the PR will accordingly result in new patch release(s). +* A library API breaking change to EITHER the provider or consumer module will result in an increase of the MAJOR version number for BOTH modules (X.y.z-provider AND X.y.z-consumer). +* A state breaking change (change requiring coordinated upgrade and/or state migration) will result in an increase of the MINOR version number for the AFFECTED module(s) (x.Y.z-provider AND/OR x.Y.z-consumer). +* Any other changes (including node API breaking changes) will result in an increase of the PATCH version number for the AFFECTED module(s) (x.y.Z-provider AND/OR x.y.Z-consumer). ## Consequences @@ -62,6 +56,7 @@ For each PR merged to main (something like this should be in our PR template): * Slightly more complexity. * This solution does not allow having provider and consumer on separate versions of e.g. the Cosmos SDK + ### Neutral ## References From 02f85768e3cfafd09af7fa4a91003b984474f2d1 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu, 24 Aug 2023 09:09:29 -0700 Subject: [PATCH 5/5] explanation on example release flow --- docs/docs/adrs/adr-012-separate-releasing.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/docs/adrs/adr-012-separate-releasing.md b/docs/docs/adrs/adr-012-separate-releasing.md index e0a4a8a470..386c8add29 100644 --- a/docs/docs/adrs/adr-012-separate-releasing.md +++ b/docs/docs/adrs/adr-012-separate-releasing.md @@ -44,6 +44,15 @@ Slightly adapting [the current semver ruleset](https://github.com/cosmos/interch * A state breaking change (change requiring coordinated upgrade and/or state migration) will result in an increase of the MINOR version number for the AFFECTED module(s) (x.Y.z-provider AND/OR x.Y.z-consumer). * Any other changes (including node API breaking changes) will result in an increase of the PATCH version number for the AFFECTED module(s) (x.y.Z-provider AND/OR x.y.Z-consumer). +### Example release flow + +We upgrade `main` to use a new version of SDK. This is a major version bump, triggering a new release for both the provider and consumer modules, `v5.0.0-provider` and `v5.0.0-consumer`. + +* A state breaking change is merged to `main` for the provider module. We release only a `v5.1.0-provider` off main. +* Another state breaking change is merged to `main` for the provider module. We release only a `v5.2.0-provider` off main. +* At this point, the latest consumer version is still `v5.0.0-consumer`. We now merge a state breaking change for the consumer module to `main`, and consequently release `v5.1.0-consumer`. Note that `v5.1.0-consumer` is tagged off a LATER commit from main than `v5.2.0-provider`. This is fine, as the consumer module should not be affected by the provider module's state breaking changes. +* Once either module sees a library API breaking change, we bump the major version for both modules. For example, we merge a library API breaking change to `main` for the provider module. We release `v6.0.0-provider` and `v6.0.0-consumer` off main. Note that most often, a library API breaking change will affect both modules simultaneously (example being bumping sdk version). + ## Consequences ### Positive