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 re-org part 1 (keepers) #1112

Merged
merged 11 commits into from
Mar 1, 2024
Merged

stakeibc re-org part 1 (keepers) #1112

merged 11 commits into from
Mar 1, 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

  • Renamed validator_section.go → rebalance.go
  • Renamed unbonding_record.go → unbonding.go
  • Created validator.go
  • Moved function into host_zone.go
  • Moved functions into epoch_tracker.go
  • Created records.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

Every function which was deleted I searched for and found where it was moved
Same thing for the tests

There were 3 net deletions which were not moved but you claim they were dead unused code
I don't think there were net logical changes but the naming of the files is better

I wish I could give better design feedback but I would need to parse how everything is calling to each other in order to do that, this was a very mechanical check. Hopefully the linter/type checker and the unit tests are enough to ensure soundness on this change.

@sampocs sampocs merged commit ccedac9 into main Mar 1, 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