-
Notifications
You must be signed in to change notification settings - Fork 122
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
docs: Create adr-012-separate-releasing.md #1229
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3922189
Create adr-011-separate-releasing.md
shaspitz 2732fe2
Update docs/docs/adrs/adr-011-separate-releasing.md
shaspitz 4447592
adr 12 not 11
shaspitz 39dbb17
correct that we use postfix not prefix
shaspitz 02f8576
explanation on example release flow
shaspitz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
--- | ||
sidebar_position: 13 | ||
title: Separate Releasing | ||
--- | ||
# ADR 012: 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 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). | ||
|
||
### 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 | ||
|
||
* 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. | ||
shaspitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* 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. | ||
* This solution does not allow having provider and consumer on separate versions of e.g. the Cosmos SDK | ||
|
||
### 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) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 don't understand how this would work in practice, can you walk me through the release process?
This is how I understood this text:
Current version is
v5.0.0
We make a statebreaking change on the consumer module that results in
v5.1.0-consumer
.The released versions now become:
v5.0.0
andv5.1.0-consumer
?Alternatively, are we releasing as
v5.0.0-consumer
andv5.0.0-provider
from the get go?Can you add a quick visualization (using a basic markdown table) that shows what happens in which scenario so the proposed change can be understood more easily?
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 explanation should help clear things up 02f8576
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.
One thing to be noted is that if we release a feature that affects both the provider and consumer, then it may end up in different versions, e.g., ICS v4.3.0-provider and v4.0.0-consumer. That would be a bit annoying, but I don't see any better way to deal with this. IMO, we should go ahead and merge 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.
if I understood well there will be no ICS versions anymore only x.y.z-provider and xy.z-consumer.
In that case it would be good to have a statement about compatibility between versions which can diverge from now on and maybe impact on testing.
From a user perspective, can I get from the versioning of provider and consumer which one 'should' work together?
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 in the long term we should assume everything is backwards compatible, similar to ibc-go. In the short term there will be exceptions (like throttling v2), but we can communicate this clearly in the versioning descriptions.