-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 a Release Process for SDK #1803
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1803 +/- ##
========================================
Coverage 63.46% 63.46%
========================================
Files 117 117
Lines 6927 6927
========================================
Hits 4396 4396
Misses 2276 2276
Partials 255 255 |
docs/RELEASE_PROCESS.md
Outdated
|
||
- [ ] 1. Decide on release designation (are we doing a patch, or minor version bump) and start a P.R. for the release | ||
- [ ] 2. Add commits/PRs that are desired for this release **that haven’t already been added to develop** | ||
- [ ] 3. Ensure that `CHANGELOG.md` contains links to issues/PRs for each item |
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 should be in the PR checklist, not the release checklist (the PR should reference the issues in the changelog entries to begin with)
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 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.
Also PENDING.md
needs to be moved to CHANGELOG.md
- combining 3/4 sounds good.
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.
PENDING.md
?
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 now use PENDING.md
for the pending changelog entries to avoid confusion for older PRs.
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.
Addressed
### `cosmos/cosmos-sdk` Release Process | ||
|
||
- [ ] 1. Decide on release designation (are we doing a patch, or minor version bump) and start a P.R. for the release | ||
- [ ] 2. Add commits/PRs that are desired for this release **that haven’t already been added to develop** |
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 might these be? Do you mean updating version/version.go
?
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 know. This addition was at the request of @jaekwon
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 OK, let's clarify.
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.
@jaekwon can you clarify the above? ☝️
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 we want to keep this as optional for stuff like version/version.go
or CHANGELOG.md
that might need fixing in the release branch?
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'm thinking that we merge PRs into develop during our normal develop process, then when it comes time to release, we create a release meta-PR forked off of develop that further merges remaining PRs. Maybe this isn't what we should be doing?
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 guess I just don't understand what the "remaining PRs" would be - why would we merge a PR to a release branch if we hadn't already merged and tested it on develop? Did you have an example in mind?
docs/RELEASE_PROCESS.md
Outdated
- [ ] 1. Decide on release designation (are we doing a patch, or minor version bump) and start a P.R. for the release | ||
- [ ] 2. Add commits/PRs that are desired for this release **that haven’t already been added to develop** | ||
- [ ] 3. Ensure that `CHANGELOG.md` contains links to issues/PRs for each item | ||
- [ ] 4. Summarize breaking API changes section under “Breaking Changes” section to the `CHANGELOG.md` to bring attention to any breaking API changes that affect RPC consumers |
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 come up with a format for this in coordination with Voyager to make sure we're communicating clearly?
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.
Brainstorming:
### Breaking Changes
#### API
- Validator struct changed. Renamed `slashes` to `punishments`
- Affected endpoints:
- /validatorsets
- /stake/candidates
#### CLI
- Renamed delegation cmd `gaiacli stake delegation -> gaiacli stake bond`
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.
Like this!!
@@ -0,0 +1,11 @@ | |||
### `cosmos/cosmos-sdk` Release Process | |||
|
|||
- [ ] 1. Decide on release designation (are we doing a patch, or minor version bump) and start a P.R. for the 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.
I think we might need a different procedure for hotfixes (namely, we might want to backport a fix to an older 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.
Yeah I think hotfix
is a usecase we didn't solve for with this 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.
I think we should have a hotfix procedure which may be (I hope 🤞) be out of scope for this PR.
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 a hotfix release should in scope cause we need to differentiate what parts of the checklist we keep.
Hotfixes should have less testing before they go out.
We don't need to several days of internal testnets.
We do need to fire a testnet and run some transactions
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're about to do a hotfix tomorrow (as per @cwgoes comment about cherry-picking 2 commits for 0.23) so lets figure it out tomorrow.
docs/RELEASE_PROCESS.md
Outdated
- [ ] 4. Summarize breaking API changes section under “Breaking Changes” section to the `CHANGELOG.md` to bring attention to any breaking API changes that affect RPC consumers | ||
- [ ] 5. Tag the commit `{{ .Release.Name }}-rcN` | ||
- [ ] 6. Kick off 1 day of automated fuzz testing | ||
- [ ] 7. Release Lead assigns 2 people to perform buddy testing script |
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.
Should we elaborate more on what this actual testing entails? (6 and 7)
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 would like to get the buddy testing script in a place where we can merge it in here and link directly to it.
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.
Once we have that finalized, can we put that in the repo somewhere? It'd be nice for everything to be in one place I think.
I've gone ahead and addressed the PR comments. I also made a change in If that is not desired, then we can go ahead and merge this as is. Would love reviews @ebuchman ! |
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.
Dope!
During a discussion with @jaekwon, @cwgoes, @ValarDragon, @jlandrews today we came up with a possible release checklist to start using when cutting SDK releases. I've added the process we agreed to in a document at
docs/RELEASE_CHECKLIST.md
. I think we should use this process (except the fuzz testing) for thev0.23
release as a test run.This would mean we need to spend tomorrow using the Plain English test "script" to ensure the functionality of the release before cutting it. We would also need buy in on the
CHANGELOG.md
updates proposed in the release process.Tagging @zmanian and @ebuchman for comment and feedback.