-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: improvements to the release process #2408
Conversation
- Network Testing: | ||
- test lab things - TBD | ||
- Infrastructure Testing: | ||
- TBD |
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.
js-ipfs currently has no deployments to internal infrastructure so there's nothing to partially roll out to here yet.
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.
Not itself, but it does rely on *-star signalling servers, preload nodes, relay nodes and bootstrappers
- [ ] IPFS Application Testing - Run the tests of the following applications: | ||
- [ ] [webui](https://github.com/ipfs-shipyard/ipfs-webui) | ||
- [ ] [ipfs-desktop](https://github.com/ipfs-shipyard/ipfs-desktop) | ||
- [ ] [ipfs-companion](https://github.com/ipfs-shipyard/ipfs-companion) |
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 success then I'd like for @lidel to publish a new version of companion to a beta channel 🙏
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 wonder if we should include ipfs-postmsg-proxy as a separate thing. Thoughts?
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.
Yes I think we should but I think it needs a big update before we can include it here....
- [ ] https://github.com/ipfs/js-ipfs/tree/master/examples/ipfs-101 | ||
- [ ] https://github.com/ipfs/js-ipfs/tree/master/examples/run-in-electron | ||
- [ ] https://github.com/ipfs/js-ipfs/tree/master/examples/running-multiple-nodes | ||
- [ ] https://github.com/ipfs/js-ipfs/tree/master/examples/traverse-ipld-graphs |
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.
A bit onerous to keep up to date, but I imagine we can easily just add a new one to this list if we notice one we forgot to add while it was being created...
- [ ] IRC | ||
- [ ] **Stage 4 - Release** | ||
- [ ] Take a snapshot of everyone that has contributed to this release (including its direct dependencies in IPFS, libp2p, IPLD and multiformats) using [this script](https://gist.github.com/alanshaw/5a2d9465c5a05b201d949551bdb1fcc3). |
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.
Eventually I'll find time to turn this into an easy to use CLI tool like I did here: https://github.com/filecoin-project/filecoin-contributors
|
||
## Performing a Release | ||
|
||
The release is managed by the "Lead Maintainer" for js-ipfs. It starts with the opening of an issue containing the content available on the [RELEASE_ISSUE_TEMPLATE](./RELEASE_ISSUE_TEMPLATE.md) not more than **48 hours** after the previous 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.
Added the 48 hours which the go-ipfs release process doc currently doesn't have. This communicates expectations for when the new release is due and will help with planning of included features. The community could even suggest inclusions.
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.
@Stebalien not sure if you want to adopt this idea too (might help with writing release notes as we go?)
doc/releases.md
Outdated
|
||
js-ipfs is currently pre-1.0. In semver terms this means [anything may change at any time](https://semver.org/#spec-item-4). | ||
|
||
However, js-ipfs follows semver "shifted by one". This means js-ipfs reserves MINOR version increments for BREAKING CHANGES and/or major feature additions and PATCH version increments for bug fixes and _backwards compatible_ minor feature additions. |
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 is what we do now. Is this working for everyone? Do we think that with the more strict release schedule we could actually use MINOR for all features (and breaks) and PATCH exclusively for bug fixes?
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 this is reasonable with a 6 week release cycle.
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 this is reasonable with a 6 week release cycle.
Just to clarify, you are voting for a change from what we do now to this?:
MINOR for all features (and breaks) and PATCH exclusively for bug fixes?
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.
Just to clarify, you are voting for a change from what we do now to this?
Yes, I'm in favor of keeping the versioning simple and predictable. It also allows us to minimize the time investment on releases, and ensure that all new features have gone through the full release testing cycle, which should help reduce the need for patch releases.
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 still isn’t going to conform to any of the tooling people have written around semver. Regardless of what we say, developers and the automation written by those developers will tend to assume that there aren’t intentional breaking changes between these releases — or they will assume that any version will break and may not adopt it because there isn’t any kind of guarantee.
I don’t see why we can’t just bump the major version number. Version numbers are essentially free, we have 2,147,483,647 of them available.
We like to talk about “1.0” as this big milestone, but we could just call that milestone something else and untangle it from version numbers. Version numbers, especially in JS, have a lot of expectations around them that we are not currently conforming to. It’s easier to adjust to the community than to try and document/educate every JS developer who finds js-ipfs
.
We’ve already done far more work to stabilize js-ipfs
and ensure there aren’t breaking changes between these releases than the average package on npm, it seems silly that we wouldn’t just adjust the versioning along with it in order to gain more adoption given that we’re putting in all of this effort to avoid breaking changes anyway.
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 agree with @mikeal, but we got here as there was a desire to make 1.0 mean something about defining an API that we want to commit to. It feels like we did that a long time ago, but still #1563 (comment)
Otherwise I am fine with patches for bug fixes, minor for everything else. Seems like it 'd be 0.40.0
then 0.40.1
when we spot the terrible regression, then 0.41.0
and 0.41.1
~6 weeks later which seems nice and predictable
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.
@mikeal - I don't think this is the right place to have that discussion. Feel free to start a separate issue, but I don't want to block merging this release process description on changing a very embedded IPFS standard - aka that 1.0 means interface reliability / forwards compatibility. Changing that current practice is a separate discussion.
I'm supportive of consistency between the js and go - aka patches should be for bug fixes or very scoped improvements - and minor should be for all new features (regardless of compatibility) such that they have testing.
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 agree with @momack2, we should have that discussion elsewhere as it's not the question on the table right now and I don't want to further expand the scope of this PR.
It sounds as if there's at least agreement that we should switch to MINOR for all features (and breaks) and PATCH exclusively for bug fixes which I'm happy to do right now and would be consistent with go-ipfs so lets do that! 🚢🚢🚢
FWIW I have a strong desire to use the major version number and I dislike that 1.0 is this big milestone for stable, production ready, "finished" to a lot of people. We lose a lot of power of semver by not using the major version number and as @mikeal pointed out many people (and machines) are still going to view anything pre-1.0 as unstable and likely to change and break, whatever convention we use for pre-1.0 versioning.
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.
sorry, wasn’t intending to block the merge, that’s why it’s just a comment and a not a full review. the change being discussed is a positive move in the right direction, i don’t see any need to block 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.
No worries - it is a necessary point to make, I'm glad someone bought it up 🙂
Lets open an issue and pick up where we left off.
|
||
### Pre-Releases and Release Candidates | ||
|
||
Prior to or during the early stages of a release cycle, js-ipfs may release a pre-release version for users who want to try out the "bleeding edge". This typically happens when a new major feature or breaking change lands in master. When this happens is entirely at the discretion of the Lead Maintainer. |
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.
Does this make sense? I've been doing this already for non-feature complete pre-releases but I realised I've not openly communicated this.
|
||
At this stage, we'll: | ||
|
||
1. Start a partial-rollout to our own infrastructure. |
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.
Again, this is a noop for js-ipfs right now but with the addition of /refs
and now GC we're one step closer to being able to be our own preload node.
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 looks good to me, just some minor suggestions.
doc/releases.md
Outdated
|
||
js-ipfs is currently pre-1.0. In semver terms this means [anything may change at any time](https://semver.org/#spec-item-4). | ||
|
||
However, js-ipfs follows semver "shifted by one". This means js-ipfs reserves MINOR version increments for BREAKING CHANGES and/or major feature additions and PATCH version increments for bug fixes and _backwards compatible_ minor feature additions. |
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 this is reasonable with a 6 week release cycle.
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.
LGTM
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 is really great!
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.
🎉
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
Also fix ToC. License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
d8193f2
to
4805832
Compare
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.
LGTM (with small URL fixes below).
- [ ] IPFS Application Testing - Run the tests of the following applications: | ||
- [ ] [webui](https://github.com/ipfs-shipyard/ipfs-webui) | ||
- [ ] [ipfs-desktop](https://github.com/ipfs-shipyard/ipfs-desktop) | ||
- [ ] [ipfs-companion](https://github.com/ipfs-shipyard/ipfs-companion) |
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 wonder if we should include ipfs-postmsg-proxy as a separate thing. Thoughts?
Co-Authored-By: Marcin Rataj <lidel@lidel.org>
Co-Authored-By: Marcin Rataj <lidel@lidel.org>
Co-Authored-By: Marcin Rataj <lidel@lidel.org>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
License: MIT Signed-off-by: Alan Shaw <alan.shaw@protocol.ai>
This all sounds good. My feeling is that six weeks is a really long time for a release - we should be able to automate pretty much everything apart from the community testing/feedback bit. For example, we should publish an RC to npm every time the head of the release branch changes (using GH actions? I don't know - @olizilla has done a bunch of stuff with automating publishing with secrets). I'm not above setting up a Jenkins box under my desk to do this if necessary. For testing dependent projects, greenkeeper already PRs new versions against dependents - it's not massively visible though, maybe we need a dashboard of which PRs are failing/unmerged. We could also split our examples into separate repos to do the same thing, or add a test suites to for them to js-IPFS itself. @alanshaw points out his FileCoin tool for grepping contributors out of the git logs of all the various deps that change in a release, great, same thing for IPFS please. We could then generate template blog posts etc (to be fleshed out by a human, obvs, but a bullet-point list of things to mention and people to thank would speed things along). I think all of this fits within the framework proposed above, it's just taking as much manual work out as possible - it would be great to bring that 6 week window down over time. |
FWIW we're going to trial this in the next js-IPFS release and may make "improvements" to the process if it doesn't work in practice. Monday 9th September is day 0, drawbridge up, feature freeze, etc. From a practical perspective this means creating a release branch with |
This is an updated release process for JS IPFS that has been largely based on the new Go IPFS release process.
It adds a release document to explain the process, adds an early testers programme and updates the release issue template.
This isn't a huge change to what we do already but does more formally specify time frames and stages. I think it makes sense to keep these very similar between the two implementations even if there are some context specific tweaks.
17th September is 6 weeks since theMonday 9th September is day 00.37
release. So we should have kicked off the process on the 27th August. I've started the release issue for0.38
(and I'd also like your input into what to include in it) but I vote we kick off this new process as soon as we reach agreement on this PR and it is merged.