-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 foldF To IorT #4075
Add foldF To IorT #4075
Conversation
Bump version to 2.8.0-SNAPSHOT to account for new symbols.
@@ -1 +1 @@ | |||
ThisBuild / version := "2.7.1-SNAPSHOT" | |||
ThisBuild / version := "2.8.0-SNAPSHOT" |
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.
Is it a typo or something?
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.
Or, sorry didn't noticed that you mentioned it in the description.
But how come we need to bump up the version here, in 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.
This PR introduces new APIs so the minor version should be incremented according to semantic versioning.
increment the MINOR version when you add functionality in a backwards compatible manner
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.
Right... But I believed it should be done once right before the release, shouldn't it?
I mean, if someone else add some other API then with this approach the version will climb to 2.9.0-SNAPSHOT even though 2.8.0
has not been released.
But nevermind, I was just surprised that we mix code changes and version management in a single PR. But if it looks OK for you, I am also fine with it. The PR itself looks good to me otherwise.
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.
@satorg if you go to change the minor version because you added APIs, you don't bump it if it ends in 0-SNAPSHOT
because you are already on a new minor version. If you are on say x.x.1-SNAPSHOT
then there is a x.x.0
release and so you should bump 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.
@satorg I'm not super familiar with the process that cats uses the manage this, but setting it to 2.8.0-SNAPSHOT
anytime prior to release (including now) should indicate that we have added at least one new symbol and if we add another one then we don't need to change the version. As opposed to 2.8.1-SNAPSHOT
which indicates the prior version was 2.8.0, thus we'd need a bump.
That said, if there is a different process I should follow here I happy to change that.
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.
Thanks @zarthross, you phrased that better than I did.
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.
@zarthross yes agree, that makes sense to me, thanks for the explanation.
@isomarcte agree, thanks for the PR!
Bump version to 2.8.0-SNAPSHOT to account for new symbols.