-
Notifications
You must be signed in to change notification settings - Fork 957
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
*: Automate various parts of the release process #2902
Comments
I would suggest we make use of the |
It's great to see this issue. I think the first step here is to document today what we do. I don't best (automation) to be the enemy of better (document what we do). Basically I view the first step is "repeatability". Then we can move to automating more and more of it. We need to make sure this isn't trible knowledge that only lives in a limited number of peoples' heads. @mxinden : in libp2p/test-plans#41 (comment), you raised the question on the priority of this. I'd like to prevent this from being an "either or" conversation. I think it should be a "both and". Something is better than nothing on this front. A couple of options for moving forward:
I think option 2 is better because it gets us validating the correctness/completeness of the release process quicker, but I defer to the team. Other ideas:
|
I've played around with Google's
Footnotes |
I took a look at cargo-release. As far as I can tell from a small dry-run cargo-release could simplify the last step of a release, namely the publishing of the many crates in dependency-then-dependent order. It would automate the |
Well that is already something! |
For the record, I did the This tool makes my life A LOT easier. Wonderful. |
That is great! What do you think of making a workflow that does what you did every time a This should be helpful, regardless of how we create that release branch, i.e. manually or with "release-please" or another tool. |
I am in favor of fully automating this. Do we know of any projects that do this today? (I am asking as I don't want rust-libp2p to be special here.) I don't have the capacity to work on this any time soon. In the meantime, I am fine continuing to do the releases. |
I haven't been following along in all the conversation, but I think it's good hygiene for a project with multiple maintainers to have a document that is clear about "how we do a release". If that document consolidates down to "run this script and you're done", that's great. I don't want us to put off expressing what we do today while we wait for the automated solution. The reasons for this ask:
|
I think the document we have is up to date with our release process. I do think we should change it though. All the manual steps like changelog entries and version bumps need to go away. It slows down my own work, esp. right after a release when all the versions are still untouched. It is also an inconvenience for contributors as we need to babysit them to make the correct changelog entries and version bumps. I am gonna work towards adopting release-please. For that, we need to fix our circular dependencies which conveniently should also improve incremental compile-times. Anyone against adopting conventional commits? That would be required to automate the changelogs and detect the correct version bumps. |
Thanks @thomaseizinger . I'm certainly supportive of automating and making things simpler. My bad (and it may just be me since I'm not making rust-libp2p commits) but I only became aware that we have a written document when you alluded to it. If anyone else is following along, the release process document is https://github.com/libp2p/rust-libp2p/blob/master/docs/release.md Concerning conventional commits, you may want to set a deadline by when folks need to respond by if they are against the idea, or otherwise move forward (i.e., no response means support). I assume this is known, but I didn't see it mentioned here: js-libp2p is using release-please I believe. @mxinden : I may be out of the loop, but doesn't our release process need a checklist item for confirming libp2p/test-plan's "big interoperability test" passes before release? |
I did not know this! Thanks for mentioning it. @achingbrain @MarcoPolo Can you comment on this?
Thanks for the encouragement. Whilst I think it is a good idea, I believe we should properly challenge it first. I'll open a separate issue to discuss: #3054
We run all interoperability tests on each pull request and (typically) only merge when they are green: https://github.com/libp2p/rust-libp2p/actions/workflows/interop-test.yml |
Thanks @thomaseizinger . Concenring interoperability tests, my understanding from a month ago was that what we run in CI is a subset of the full interoperability testsuite (because the full suite takes too long). @laurentsenta : I'm sorry to pull you in here. I believe this has been discussed before. I looked in https://github.com/libp2p/test-plans/issues for a discussion on this, but didn't find one. Please correct me too if there is no longer a "big tests" that should be run manually for a release. |
Correct. We currently only run the go-rust-interop-latest composition test instead of the go-rust-interop composition test. rust-libp2p/.github/workflows/interop-test.yml Lines 26 to 32 in b42f286
https://github.com/libp2p/test-plans/blob/master/ping/_compositions/go-rust-interop.toml @laurentsenta do you know off the top of your head how much longer the full test suite would take? If it is not too much time, my preference would be to simply run the full one on rust-libp2p's pull request CI instead of adding a new step to the release process. |
@mxinden the |
Status update: We now enforce that each PR uses conventional commits: #3204 We also enforce that every PR makes the according semver bump with its API changes: #2647. This has the benefit that we detect breaking changes in our APIs very early. This however has the downside that we need to do the version bump within the PR that makes the breaking change which is one of the things I've been trying to move away from. Tools like cargo release version --package libp2p-core minor --execute --no-confirm I opened crate-ci/cargo-release#628 which would be exactly what we need. In case that feature request gets declined, I am thinking of scripting something together based on Input welcome. I am sure this process can be further improved. |
We discussed this in our open maintainers call and @rkuhn suggested that instead of failing CI on detected breaking changes via Only applying the label would make us aware that the a PR breaks the API but we can then defer the actual bump of versions to a release pull request. This would allow us to consider a tool like That said, it wouldn't be too difficult to do it manually with some repeated invocations of |
Perhaps a good combination of automation would be to:
(3) could potentially be automated with a script. |
Heads-up that development has started on a v2 of the cargo-semver-checks GitHub Action (obi1kenobi/cargo-semver-checks-action#21). A new feature we're considering for v2 is the ability to apply labels to PRs corresponding to their semver-required minimum version bump (e.g. Would that be useful here? If so, would you be interested in being an early adopter and sharing your feedback? Of course, everyone working on cargo-semver-checks is an unpaid volunteer at the moment, so I can't promise a specific timeline for when this new feature might be available. |
It might be useful yes! Bear in mind that we are still figuring things out here ourselves too so take any feedback with a grain of salt :) |
The perfect processIn a perfect world, I think this would be a good process:
LimitationsThis has some limitations though:
(1) warrants some kind of human override, ideally at the time we write the PR. Human override for next version bumpOne solution for (1) could be to have a configuration file in our repository that looks something like: {
"libp2p-swarm": "minor",
"libp2p-gossipsub": "major",
} We would maintain this file between releases, i.e. it gets cleared every time we release a version. This file would be used by a script at release time that:
This would allow us to fairly easily control the next version bump of a crate during the PR process. Manual changelog entriesInstead of amending the actual changelog file (which often causes merge conflicts), we could employ a strategy where the changelog notes for a particular PR are added as separate files. I've seen projects that have a This would be a much more automation friendly approach and would cause less conflicts. Additionally, it would be easy to write a lint that ensures we add a changelog entry for a crate as soon as any source-code in that crate is touched. ConclusionWe've set out this journey with the goal of "let's not reinvent the wheel" and I think it is a novel goal. However, it seems that our particular combination of squash merging PRs, big workspaces and the requirement of "brain-less" releases doesn't fit any of the existing tools. I think they are all good requirements so I am reluctant to change any of them for the sake of fitting our process into a tool, hence I am not strictly opposed to rolling our own solution especially when it is mostly plumbing work. |
Had a chat with @mxinden today about this. We definitely want to keep the PR squash-merging strategy. Unfortunately this means that pretty much every tool / strategy that extracts changelog or version information from commits is out the window because it is just not granular enough after we squash merge the commit history of a PR. We also definitely want to retain the "ease" of releases in terms of figuring out the next version number. Currently, we achieve this by bumping the version as part of the PR where we have the most context on whether or not the change is breaking. In an ideal world, we would only bump the version at release time, meaning we need to somehow retain the information whether the PR is breaking. Regardless of when we bump the version, what we definitely need is a script that:
(1) and (2) can be handled by The first step in moving forward with this issue is to write a script that does the above 3. I created an issue that tracks the creation of this script here: #3348 Footnotes
|
In a 2nd step, I'd like to move away from bumping versions as part of the pull request and instead only capture what bump is necessary given the changes since the last release. In #2902 (comment), I suggested to use an external configuration file but I think we can do much better! Cargo has support for arbitrary config values as part of the [package.metadata.semver]
next-bump-level = "minor" This has several advantages:
Changelog entries would be added as they are currently with the only difference that the top heading would always say "unreleased" until we actually release a version. What do you think about this idea @libp2p/rust-libp2p-maintainers? I think it is quite lean and would allow for a very clean automation of the entire version bumping requirement. |
The story of #3312 (comment) is another data point towards not bumping versions in PRs. It hides from us, which changes would be breaking in lieu of what is already merged. |
If you decide to go the metadata route, you can use the value of the |
Nice! This synergy really makes me want to go for this solution :) Do I understand correctly that |
This makes me think we might want to go back to the idea of structured changelogs:
# file: changelogs/3254.changelog
semver-bump = "minor"
changelog-text = '''
This is my changelog text.
'''
|
Yes, exactly. It says "assume this is a release" and semver-checks it as such. |
Actually, what we should be tracking is |
Once crates have published a 1.0 version, this might not be enough — assuming you care about "deprecations/
|
Yep, you are right! I started building a tool here: https://github.com/thomaseizinger/semverlog There, a deprecation will correctly cause a minor bump once the crate version is > 1.0. |
I decided to go a slightly different route in regards to the config. A "major" would always mean bumping X in |
Consider a slight terminology switch: |
I am not using either terminology in the file format: https://github.com/thomaseizinger/semverlog#change-file-format. Instead I am talking about breaking changes. A breaking change will always bump the leftmost non-zero number. For the |
@mxinden Would you mind having a look at https://github.com/thomaseizinger/semverlog? I am curious to know what you think. The workflow I am envisioning is:
What this does not do is figure out the order in which the crates need to be released. In our release docs, we currently do make use of I think we would need some integration from |
Alternatively, it shouldn't be too difficult to add an xtask to our workspace that figures out which crates need releasing and in which order. Even with our solution of holding back breaking changes in a milestone, the changelog files attract a lot of merge conflicts. See #3590 (comment) for example. |
@mxinden now that we are following #3532, what do you think of adding another piece of automation for our releases? Following the spirit of conventionalcommits (although not quite following the spec), we could start prefixing release PRs with The release PRs would have to - as they already do - prepare the final steps for the release, i.e. trim the At least for patch releases, this could be a quick win and move us closer to an automated setup. I do think that the philosophy here is to have many small pieces of automation that are triggered in series such that we can "continue" or trigger the process at any point. |
This would be the ideal workflow for me. |
I see two risks that I'd like to discuss:
With our current setup, we would trigger a release every time such a PR would be merged into In my previous job, we solved this using Do you think any of these risks are worth dealing with? At the end of the day, we could always yank an accidentally released crate or make another release on top that fixes something. |
The only risk here is that we release something too early. I.e. that we release a crate even though we still wanted to pack more changes into that release through pending pull requests. Given that we bump versions with every change, at least we won't make any semver mistakes. Unless I am missing something, I don't think we need to protect against this. I don't think our repository is busy enough for us to face significant race conditions. Am I missing something? |
I also don't think it is busy enough. It would just be one thing that we have to "think about" when it comes to releases and it would be nice to keep the effort as low as possible. |
I am closing this in favor of continuing the discussion in #4541. |
Whilst libp2p#3312 was in development, we pushed a new release out and forgot to move the changelog entries to the new version. Unfortunately, this is all still very manual until we have a solution for libp2p#2902 so this stuff keeps happening. Pull-Request: libp2p#3541.
Description
Most tasks around releasing crates in our monorepo are currently performed manually:
Motivation
Repetitive tasks consume unnecessary time and should be automated.
Requirements
libp2p-core
by a minor version, all dependent crates should update their dependency automatically and bump their on version as required.CHANGELOG
links to what the header will be after the release, meaning until then, they are all broken because the contain the-unreleased
postfix.It would be nice to trigger these operations through a bot / CI so contributors don't have to learn how to do it.
Open questions
Are you planning to do it yourself in a pull request?
Maybe.
The text was updated successfully, but these errors were encountered: