-
-
Notifications
You must be signed in to change notification settings - Fork 54
Feature/589 windows start when available #590
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: master
Are you sure you want to change the base?
Feature/589 windows start when available #590
Conversation
WalkthroughThis pull request adds a Windows-only scheduling configuration option Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
For feature #589 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/content/schedules/configuration.md (1)
204-211: Consider minor grammar improvement.The documentation is clear and helpful. However, consider adding a comma before "but" in line 208 for improved readability.
🔎 Proposed fix
-For example, if a backup is scheduled for 3:00 AM but the computer is off, enabling this option will run the backup when the computer is next available. +For example, if a backup is scheduled for 3:00 AM, but the computer is off, enabling this option will run the backup when the computer is next available.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
config/profile.go(1 hunks)config/schedule.go(3 hunks)docs/content/schedules/configuration.md(1 hunks)docs/content/schedules/task_scheduler/index.md(1 hunks)schedule/config.go(1 hunks)schedule/handler_windows.go(1 hunks)schedule/handler_windows_test.go(1 hunks)schedule_jobs.go(1 hunks)schtasks/config.go(1 hunks)schtasks/taskscheduler.go(1 hunks)schtasks/taskscheduler_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-29T16:14:02.636Z
Learnt from: zumm
Repo: creativeprojects/resticprofile PR: 541
File: schtasks/taskscheduler.go:60-67
Timestamp: 2025-07-29T16:14:02.636Z
Learning: In the schtasks package, `config.Command` is guaranteed to be just a path to executable, making single quotes sufficient for wrapping in command construction.
Applied to files:
schedule/handler_windows.goschtasks/config.go
🧬 Code graph analysis (5)
schedule/handler_windows.go (2)
shell/command.go (1)
Command(37-51)schtasks/principal.go (1)
RunLevel(30-30)
config/profile.go (1)
util/maybe/bool.go (1)
Bool(11-13)
schtasks/taskscheduler_test.go (4)
schtasks/config.go (1)
Config(5-14)calendar/event.go (2)
NewEvent(27-42)Event(15-24)schtasks/task.go (2)
RegistrationInfo(16-22)Task(24-33)schtasks/settings.go (1)
Settings(9-27)
schtasks/taskscheduler.go (1)
schtasks/settings.go (1)
Settings(9-27)
config/schedule.go (1)
util/maybe/bool.go (1)
Bool(11-13)
🪛 LanguageTool
docs/content/schedules/configuration.md
[uncategorized] ~208-~208: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le, if a backup is scheduled for 3:00 AM but the computer is off, enabling this opti...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (12)
schtasks/config.go (1)
13-13: LGTM!The
StartWhenAvailablefield is correctly added to theConfigstruct and aligns with the Windows Task Scheduler's Settings structure.config/profile.go (1)
329-329: LGTM!The
ScheduleStartWhenAvailablefield is correctly configured with appropriate tags and follows the established pattern for optional Windows-specific schedule settings.schtasks/taskscheduler.go (1)
187-187: LGTM!The assignment correctly maps the configuration value to the Windows Task Scheduler settings.
schedule/handler_windows.go (1)
79-88: LGTM!The
StartWhenAvailablefield is correctly propagated from the job configuration to theschtasks.Configstructure. The alignment adjustments maintain code consistency.schedule/config.go (1)
28-28: LGTM!The
StartWhenAvailablefield is appropriately added to the scheduling configuration structure.config/schedule.go (3)
50-50: LGTM!The
StartWhenAvailablefield is correctly added toScheduleBaseConfigwith appropriate type and tags.
104-106: LGTM!The initialisation logic for
StartWhenAvailablefollows the established pattern for optional boolean fields.
123-123: LGTM!The override logic correctly propagates the
StartWhenAvailablevalue from the section configuration.schedule_jobs.go (1)
242-242: LGTM!The
StartWhenAvailablefield is correctly populated by converting themaybe.Boolvalue to a boolean usingIsTrue(), consistent with theHideWindowfield pattern.docs/content/schedules/task_scheduler/index.md (1)
38-49: LGTM! Clear and helpful documentation.The documentation clearly explains the purpose of the
schedule-start-when-availableoption and provides a practical example. The mapping to Windows Task Scheduler terminology is helpful for users familiar with that interface.schtasks/taskscheduler_test.go (1)
291-343: LGTM! Thorough test coverage.The test properly verifies that the
StartWhenAvailablesetting:
- Is correctly applied when creating the task definition (line 316)
- Persists when the task is created in Windows Task Scheduler (line 322)
- Is correctly exported back from Task Scheduler (line 342)
The test follows the established pattern and includes proper cleanup.
schedule/handler_windows_test.go (1)
82-108: LGTM! Appropriate integration test.The test follows the established pattern from
TestHideWindowOptionand properly verifies that a job can be created with theStartWhenAvailableoption. The detailed verification of the setting itself is appropriately handled at theschtaskslayer (inTestStartWhenAvailableOptionintaskscheduler_test.go), whilst this test ensures the integration works correctly.
At first glance it didn't do too badly. Which model did you use? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #590 +/- ##
==========================================
+ Coverage 81.00% 81.04% +0.04%
==========================================
Files 137 137
Lines 11083 11089 +6
==========================================
+ Hits 8977 8986 +9
+ Misses 1680 1677 -3
Partials 426 426
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
claude opus 4.5 |
Mostly generated with ai from a script that I had to solve the same issue. First time with a go repository.