-
Notifications
You must be signed in to change notification settings - Fork 64
backup: improve write meta logic #857
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
base: main
Are you sure you want to change the base?
backup: improve write meta logic #857
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: huanghaoyuanhhy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/kind improvement |
d94728d to
17ece2d
Compare
17ece2d to
c0c1b02
Compare
santiago-wjq
left a comment
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.
PR Review: backup: improve write meta logic
Summary
This PR refactors the writeMeta function in core/backup/task.go, replacing repetitive meta build/write code blocks with a loop-based approach.
Changes Analysis
Before (~47 lines): 5 nearly identical code blocks for each meta type
After (~20 lines): Single loop iterating over a slice of meta entries
Pros ✅
- Reduces code duplication - Eliminates 5 repetitive code blocks
- Easier to maintain - Adding a new meta type only requires adding one line to the slice
- Consistent error handling - All meta types use the same error format pattern
Suggestions ⚠️
-
Naming: The struct name
metaTypesis slightly misleading since it represents a single meta entry, not multiple types. Consider renaming tometaEntryormetaWriter:type metaEntry struct { Type mpath.MetaType Fn func() ([]byte, error) }
-
Logger inconsistency: The function starts with
t.logger.Info("start write meta")but ends withlog.Info("finish write meta"). Consider usingt.loggerconsistently for both.
Overall
Good refactoring that follows the DRY principle. The logic is correct and functionality remains unchanged. 👍
Signed-off-by: huanghaoyuanhhy <[email protected]>
c0c1b02 to
72e0687
Compare
No description provided.