refactor(backup): use time.Duration for backup interval end-to-end#1150
Merged
Conversation
Replace IntervalMinutes int with Interval time.Duration in Config, migrate --git-backup-interval flag from fs.Int (minutes) to fs.Duration so operators can use duration strings like 60m or 2h, and remove the redundant interval parameter from NewScheduler so Config is the single source of truth. Negative and sub-minute values are now guarded with an explicit warning and written back to cfg.Interval so the stored config always reflects what the scheduler actually uses.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the git-backup scheduler configuration to use time.Duration end-to-end, making the backup interval configurable via standard Go duration strings and removing redundant interval plumbing so the repo config becomes the single source of truth for scheduling.
Changes:
- Replace
Config.IntervalMinutes intwithConfig.Interval time.Durationand remove the oldDuration()helper. - Update the backup scheduler to read/normalize the interval from
repo.cfg.Interval(including clamping negative and sub-minute values). - Migrate
--git-backup-intervalfrom an integer-minutes flag tofs.Duration, and update tests accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/backup/scheduler.go | Scheduler now reads/normalizes repo.cfg.Interval and no longer accepts a separate interval parameter. |
| internal/backup/scheduler_test.go | Updates scheduler tests to use Config.Interval time.Duration and new NewScheduler signature. |
| internal/backup/config.go | Switches backup config interval to time.Duration and removes the minutes-to-duration helper. |
| cmd/leafwiki/main.go | Updates CLI usage + flag parsing to accept duration strings for --git-backup-interval and passes duration into backup config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add two scheduler tests that verify the new clamping behavior: negative intervals are set to 0 (manual-only, no ticker), and sub-minute intervals are raised to minInterval with the corrected value written back to cfg.Interval.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace IntervalMinutes int with Interval time.Duration in Config, migrate --git-backup-interval flag from fs.Int (minutes) to fs.Duration so operators can use duration strings like 60m or 2h, and remove the redundant interval parameter from NewScheduler so Config is the single source of truth. Negative and sub-minute values are now guarded with an explicit warning and written back to cfg.Interval so the stored config always reflects what the scheduler actually uses.