chore: upgrade Go version to 1.25 #564
Conversation
WalkthroughBumps Go toolchain to 1.25 across workflows and go.mod, enables godoclint, adjusts tests in lock package to use context timeouts and global signal handling, modifies a space-writing loop in commands_display.go, adds a new DisplayStatus method for systemd, changes a Windows handler method signature, and updates several comments/docstrings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as Caller
participant HS as HandlerSystemd
participant SYS as getSystemdStatus
participant Out as Stdout
U->>HS: DisplayStatus(profileName)
HS->>SYS: getSystemdStatus(profileName, root/user scope)
alt status text returned
SYS-->>HS: status (non-empty)
HS->>Out: print "Timers summary" + status
HS-->>U: nil
else no status or error
SYS-->>HS: "" or error
note right of HS: Returns nil without output when empty/error
HS-->>U: nil
end
sequenceDiagram
autonumber
actor T as Test
participant C as context.WithTimeout(5s)
participant P as exec.CommandContext
participant Proc as Child process
T->>C: create ctx,cancel
T->>P: start with ctx
P->>Proc: run
alt completes before timeout
Proc-->>T: exit
else timeout
C-->>Proc: context deadline -> termination
Proc-->>T: exit due to ctx
end
T->>C: cancel() (defer)
sequenceDiagram
autonumber
actor Main as lock/test/main.go
participant OS as os/signal
participant L as Lock
participant Tm as time.After
Main->>OS: Notify(sigChan, Interrupt,SIGTERM,SIGABRT)
Main->>L: Acquire
L-->>Main: "lock acquired"
par wait
Main->>OS: receive from sigChan
and timeout
Main->>Tm: time.After(d)
end
alt signal received
Main-->>Main: handle and release
else timeout fires
Main-->>Main: release on timeout
end
Main->>OS: signal.Stop(sigChan) (defer)
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
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #564 +/- ##
=======================================
Coverage 79.69% 79.69%
=======================================
Files 137 137
Lines 13467 13467
=======================================
Hits 10732 10732
Misses 2310 2310
Partials 425 425
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:
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
term/term.go (1)
89-89: Fix incorrect doc comment phrasing.Docstring still claims the function “returns true”, but the function returns the (width, height) pair. Please align the comment with the actual return values.
-// OsStdoutTerminalSize returns true as os.Stdout is a terminal session +// OsStdoutTerminalSize returns the current width and height of os.Stdout.flags.go (1)
61-62: Guard the type assertion to avoid possible panics
castwill usually return the expected concrete type, but if a future caller uses a named type (type CmdOutput string,type Verbosity int, etc.),v.(T)will panic despite a successful conversion. Wrapping the assertion with anokcheck keeps the helper robust and removes the need for the lint suppression.- if err == nil { - defaultValue = v.(T) //nolint:forcetypeassert + if err == nil { + if converted, ok := v.(T); ok { + defaultValue = converted + } else { + clog.Errorf("cannot assign env variable %s=%q to expected type %T", key, value, defaultValue) + }commands_display.go (1)
401-403: Reduce syscalls: write all spaces in one callWriting one byte per iteration is inefficient on hot paths. Prebuild the run of spaces and write once.
Apply:
- for i := 0; i < l.breakLength; i++ { - _, _ = l.writer.Write(l.tokens[0:1]) // fill spaces for alignment - } + if l.breakLength > 0 { + buf := bytes.Repeat(l.tokens[0:1], l.breakLength) + _, _ = l.writer.Write(buf) // fill spaces for alignment + }schedule/handler_systemd.go (1)
96-121: Silent error swallow and brittle "0 timers" checkCurrently any error from
getSystemdStatusis silently ignored, making diagnoses hard; and matching"0 timers"is locale‑dependent.Minimal improvement while keeping UX silent:
- if err != nil || status == "" || strings.Contains(status, "0 timers") { - // fail silently - return nil //nolint:nilerr - } + if err != nil { + clog.Debugf("getSystemdStatus(%q) error: %v; output: %q", profileName, err, status) + return nil //nolint:nilerr + } + if status == "" || strings.Contains(status, "0 timers") { + return nil + }Optional follow‑up: replace the substring check with a small helper that counts non‑header lines to avoid locale coupling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.github/workflows/build.yml(1 hunks).github/workflows/codeql.yml(1 hunks).github/workflows/doc.yml(1 hunks).github/workflows/docker.yml(1 hunks).github/workflows/release-doc.yml(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/snapshot.yml(1 hunks).golangci.yml(1 hunks)commands_display.go(1 hunks)darwin/calendar_interval.go(2 hunks)flags.go(1 hunks)go.mod(1 hunks)lock/lock_test.go(1 hunks)lock/test/main.go(1 hunks)mask/mask.go(1 hunks)schedule/config.go(1 hunks)schedule/handler_systemd.go(2 hunks)schedule/handler_windows.go(2 hunks)schtasks/principal.go(2 hunks)schtasks/settings.go(1 hunks)schtasks/taskscheduler.go(1 hunks)shell/arg.go(2 hunks)term/term.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-02-14T22:53:42.689Z
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#459
File: schtasks/schtasks.go:29-30
Timestamp: 2025-02-14T22:53:42.689Z
Learning: In the `schtasks` package, `tasksPath` is defined as a constant with value `\resticprofile backup\` in `taskscheduler.go`. It's used as a prefix for managing task paths in the Windows Task Scheduler.
Applied to files:
schtasks/taskscheduler.go
📚 Learning: 2025-02-04T14:38:07.701Z
Learnt from: creativeprojects
PR: creativeprojects/resticprofile#425
File: schedule/handler_windows.go:97-118
Timestamp: 2025-02-04T14:38:07.701Z
Learning: The shell.SplitArguments function in the resticprofile project returns only []string and does not return any error.
Applied to files:
go.mod
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test (1.25, ubuntu-latest)
- GitHub Check: Build and test (1.25, macos-latest)
- GitHub Check: Build and test (1.25, windows-latest)
🔇 Additional comments (10)
lock/test/main.go (1)
23-27: Broader signal coverage at start-up looks solidCatching INT/TERM/ABRT at process start and deferring
signal.Stopkeeps the handler active for the full lifecycle without leaking subscriptions on early exit. Happy with this.lock/lock_test.go (2)
274-295: Timeout guard shieldsTestLockIsRemovedAfterInterruptSignalWrapping the helper process in a 5 s context and tightening the failure messages should prevent future hangs while keeping the intent clear. Looks good to me.
302-323: Shell-based interrupt test hardening looks goodThe context-bound command and explicit assertions mirror the previous test nicely, giving us the same protection against stalled children in the shell variant. No issues spotted.
.github/workflows/codeql.yml (1)
56-57: Go 1.25 specification in CodeQL workflow looks goodUpdating the runner to pick up Go ≥1.25 keeps the static analysis aligned with the toolchain bump elsewhere in the repo. Nicely done.
.github/workflows/release-doc.yml (1)
22-23: Documentation release job now matches the Go toolchain upgradeThe
setup-gostep following the~1.25constraint keeps this workflow consistent with the rest of the CI surface..github/workflows/release.yml (1)
23-24: Release workflow aligned with Go 1.25This keeps the release pipeline on the same toolchain level as the rest of the project—looks good.
.golangci.yml (1)
25-25: Enablinggodoclintstrengthens lint coverageNice to see the documentation linter turned on; having it active at the CI level will keep exported symbols properly documented.
schedule/handler_systemd.go (1)
322-325: Doc comment capitalisation LGTMComment now starts with the function name; good for godoc.
schedule/handler_windows.go (2)
142-145: Doc comment tidy LGTMCorrect capitalisation for godoc.
107-117: API alignment confirmed
TheHandlerinterface, all platform implementations and call sites (including tests) correctly referenceDisplayJobStatusandDisplayStatus.
Also try to fix a flaky unit test on my local machine