Skip to content
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

chore(deps): bump core version to v1.21.2-tm-v0.34.27 #1878

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Jun 5, 2023

Overview

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id added the chore optional label for items that follow the `chore` conventional commit label Jun 5, 2023
@rach-id rach-id requested a review from evan-forbes as a code owner June 5, 2023 14:46
@rach-id rach-id self-assigned this Jun 5, 2023
@MSevey MSevey requested a review from a team June 5, 2023 14:46
@rootulp
Copy link
Collaborator

rootulp commented Jun 5, 2023

[question] if https://github.com/celestiaorg/celestia-core/releases/tag/v1.21.2-tm-v0.34.27 only contains a feature, should it have been released as v.1.22.0-tm-v0.34.27 instead?

@rach-id
Copy link
Member Author

rach-id commented Jun 5, 2023

v.1.22.0-tm-v0.34.27 according to semver, yes. But from what I noticed, the pattern we follow is something like:

  • breaking change: we increment the minor version
  • features/bug fixes: we increment the bug version

While the major version stays the same.

I can update if I'm wrong tho

@rootulp
Copy link
Collaborator

rootulp commented Jun 5, 2023

@cmwaters @evan-forbes what are your thoughts on #1878 (comment) ? If that's the case, I'd like to update https://github.com/celestiaorg/celestia-core#versioning b/c it's not obvious to me and I would've cut the wrong release in this instance.

@evan-forbes
Copy link
Member

evan-forbes commented Jun 6, 2023

I know our forks of the cosmos-sdk and tendermint don't follow semver, and we should try to as hard as possible to follow semver imo

@rootulp I think the confusion for me might have just been that celestiaorg/celestia-core#1020 maybe should have just been a refactor or even fix, since afaiu we didn't break or add any apis we just fixed the implementation unecessarily needed the cached txs. Technically, we added an exported struct, but its only returned or accepted by unexpectored functions.

While it says feat, in my head the representation of that change is not breaking anything so bumping the patch made sense. I'm fine with keeping it the way it is, but we should try to be correct with how we prefix our PRs to make this process easier

@rootulp
Copy link
Collaborator

rootulp commented Jun 6, 2023

and we should try to as hard as possible to follow semver imo

Agreed so seems like no changes are needed to https://github.com/celestiaorg/celestia-core#versioning

+1 to being careful with our PR conventional commit prefixes because:

  • Automatically determining a semantic version bump (based on the types of commits landed).
  • Communicating the nature of changes to teammates, the public, and other stakeholders.

source. I could see how this particular scenario is a gray area on which prefix to use.

@rach-id
Copy link
Member Author

rach-id commented Jun 6, 2023

but we should try to be correct with how we prefix our PRs to make this process easier

My bad, I used feat because I was unsure whether it's just a refactor or a feature. Because, it is a feature from the perspective that it changes a requirement for validators to run orchestrators. However, it doesn't make any change to the API or anything.

@rach-id rach-id merged commit bb00764 into celestiaorg:main Jun 6, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore optional label for items that follow the `chore` conventional commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants