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

stakeibc file re-org part 3 (ibc, ica, icq, and utilities) #1116

Merged
merged 23 commits into from
Mar 2, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Mar 1, 2024

Context

Stakeibc has grown quite large over the years. This PR does a file re-org cleanup to better organize functions and remove ignite tech debt. This should make it easier to onboard new devs to the codebase.

Note to reviewer: There were no logical changes made in this PR. It only copy pasted functions to different files. When reviewing, I'd recommend scrolling each commit and making sure all functions are accounted for during the move.

Brief Changelog

Logically grouped ibc/ica/icq code and other utilities into the following files:

  • interchainaccounts.go
  • interchainqueries.go
  • ibc.go
  • redemption_rate.go
  • invariants.go

Copy link
Contributor

@ethan-stride ethan-stride left a comment

Choose a reason for hiding this comment

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

LGTM
Made sure any removed function was added somewhere else, walked the commits this time which made it easier to conceptualize.
I semi-disagree with rename of update_redemption_rate_test to just redemption_rate_test unless you are going to add other tests around RR into the renamed file later. Better to be specific, if you want the name to be shorter do update_RR_test or something

@sampocs
Copy link
Collaborator Author

sampocs commented Mar 2, 2024

we'll add more tests to that file

@sampocs sampocs merged commit 16a15d4 into main Mar 2, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants