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.

Sign up for GitHub

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

Jump to bottom

chore: v0.25.2 #3145

Closed
wants to merge 7 commits into from
Closed

chore: v0.25.2 #3145

wants to merge 7 commits into from

Conversation

aljo242
Copy link
Contributor

@aljo242 aljo242 commented Nov 21, 2022

This update bumps ibc-go and cosmos-sdk to their latest versions as per this security advisory.

@tbruyelle
Copy link
Contributor

tbruyelle commented Nov 21, 2022 •
edited
Loading

Side note about release fix process:

  • the base branch (created from the tag) should be named v0.25.x (or fix/v0.25.x or release/v0.25.x), so you don't need to create a new one if there's an other fix to do. Here you named it release/v0.25.2-final, so if there's an other fix, you will have to create a new branch or it will be confusing.
  • the fix branch can be named as usual, like fix/this-bug, I don't think it matters to quote the current version.
  • the following tags should be located on the base branch (for that I don't know what was your plan).

Finally for the backport, you can probably merge the base branch to main (but w/o deleting it), it should do the trick.

Ofc these are suggestions that follow the model I advised in #2930, we can discuss about something else if the team wants something different.

@aljo242
Copy link
Contributor Author

aljo242 commented Nov 21, 2022

Side note about release fix process:

  • the base branch (created from the tag) should be named v0.25.x (or fix/v0.25.x or release/v0.25.x), so you don't need to create a new one if there's an other fix to do. Here you named it release/v0.25.2-final, so if there's an other fix, you will have to create a new branch or it will be confusing.
  • the fix branch can be named as usual, like fix/this-bug, I don't think it matters to quote the current version.
  • the following tags should be located on the base branch (for that I don't know what was your plan).

Finally for the backport, you can probably merge the base branch to main (but w/o deleting it), it should do the trick.

Ofc these are suggestions that follow the model I advised in #2930, we can discuss about something else if the team wants something different.

Yeah i should have done fix/update-sdk -> release/v0.25.2

@tbruyelle
Copy link
Contributor

Yeah, you're right. I did this incorrectly. Will do it right next time !

I looked at the schema I roughly drafted for #2930 and yes the base branch name is confusing, it's not possible to guess if the x is a placeholder or is part of the name.

@tbruyelle
Copy link
Contributor

tbruyelle commented Nov 21, 2022 •
edited
Loading

Yeah i should have done fix/update-sdk -> release/v0.25.2

Actually : fix/update-sdk -> release/v0.25.x

Where the base branch is created like that

$ git branch release/v0.25.x v0.25.1

(should be v0.25.0 but we started the new model after v0.25.1 so...)

tbruyelle
tbruyelle reviewed Nov 21, 2022
View reviewed changes
ignite/pkg/cosmoscmd/root.go
@@ -382,7 +382,7 @@ func (a appCreator) newApp(
baseapp.SetIndexEvents(cast.ToStringSlice(appOpts.Get(server.FlagIndexEvents))),
baseapp.SetSnapshot(snapshotStore, snapshotOptions),
baseapp.SetIAVLCacheSize(cast.ToInt(appOpts.Get(server.FlagIAVLCacheSize))),
baseapp.SetIAVLDisableFastNode(cast.ToBool(appOpts.Get(server.FlagIAVLFastNode))),
baseapp.SetIAVLDisableFastNode(cast.ToBool(appOpts.Get(server.FlagDisableIAVLFastNode))),
Copy link
Contributor

@tbruyelle tbruyelle Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point where does this come from plz ?

Because I remember we added the red line for a recent cosmos bug fix, also, and now we somehow disable it right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag was introduced in sdk v0.46.3. But then they changed the name of the flag.

FlagIAVLFastNode -> FlagDisableIAVLFastNode. So this change is just propagating that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok only name change fine, thx!

@aljo242
Copy link
Contributor Author

aljo242 commented Nov 21, 2022

I will open a new PR with the proper structure.

@aljo242
Copy link
Contributor Author

aljo242 commented Nov 21, 2022

#3146

@aljo242 aljo242 closed this Nov 21, 2022
@aljo242 aljo242 deleted the release/v0.25.2 branch November 22, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbruyelle tbruyelle tbruyelle left review comments

@ilgooz ilgooz Awaiting requested review from ilgooz

@lumtis lumtis Awaiting requested review from lumtis

@jeronimoalbi jeronimoalbi Awaiting requested review from jeronimoalbi

@fadeev fadeev Awaiting requested review from fadeev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aljo242 @tbruyelle