This repository was archived by the owner on Feb 12, 2024. It is now read-only.
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.
refactor: improvements to the release process #2408
refactor: improvements to the release process #2408
Changes from 8 commits
4229a14
d929e46
fef01c1
19a7a5f
f0ab4d3
ed3b138
7a296a7
4805832
d2965dc
8680975
1754255
c5b86b2
8af5edc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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
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....
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...
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
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.
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?)
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.
Just to clarify, you are voting for a change from what we do now to 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.
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
then0.40.1
when we spot the terrible regression, then0.41.0
and0.41.1
~6 weeks later which seems nice and predictableThere 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.
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.