-
Notifications
You must be signed in to change notification settings - Fork 13
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
Refactor to simplify merging child slabs in array #525
base: main
Are you sure you want to change the base?
Conversation
Currently, ArrayMetaDataSlab.MergeOrRebalanceChildSlab() is complicated with over 300 lines of code. This commit refactors merging aspect of this function by adding new two helper functions. This commit also uses go1.21 slices package to simplify slice operations.
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.
Nice work refactoring into smaller and reusable functions!
I assume the code was extracted using automatic refactoring (e.g. IDE)? I didn't fully check if it matches exactly the old code (given the diff is hard to read)
@turbolent Yeah, the diff for this PR is hard to read. The code was extracted with a combination of manual and automatic refactoring.
I re-reviewed this PR and have been running brief smoke tests (on my desktop) for PRs with these types of changes and no problems were found. After the remaining 10 PRs for issue #464 are merged I will:
Thanks again for these PR reviews and questions! 👍 |
Thanks for the explanation and double checking! 👍 |
Updates #464
Currently,
ArrayMetaDataSlab.MergeOrRebalanceChildSlab()
is complicated with over 300 lines of code.This PR refactors merging in this function by adding new two helper functions
mergeChildren()
andupdateChildrenHeadersAfterMerge()
.This PR also uses go1.21
slices
package to simplify slice operations.main
branchFiles changed
in the Github PR explorer