Skip to content

fix: fixed log rotation not using the wrapper - #207

Merged
parkervcp merged 1 commit into
pelican:mainfrom
QuintenQVD0:logrotate
Aug 13, 2026
Merged

fix: fixed log rotation not using the wrapper#207
parkervcp merged 1 commit into
pelican:mainfrom
QuintenQVD0:logrotate

Conversation

@QuintenQVD0

@QuintenQVD0 QuintenQVD0 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Changed the logger to use the rotation-aware handler instead of the file descriptor

Summary by CodeRabbit

  • Bug Fixes
    • Improved CLI logging by ensuring log output is written through the configured rotating log handler.
    • Helps preserve expected log rotation behavior during command-line use.

@QuintenQVD0
QuintenQVD0 requested a review from a team as a code owner August 12, 2026 16:44
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 921619b9-b10b-4224-b949-c6af2eb75df8

📥 Commits

Reviewing files that changed from the base of the PR and between d222d85 and 3964414.

📒 Files selected for processing (1)
  • cmd/root.go
📜 Recent review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: Build and Test (ubuntu-22.04, 1.26.5, linux, amd64)
  • GitHub Check: Build and Test (ubuntu-22.04, 1.25.12, linux, amd64)
  • GitHub Check: Build and Test (ubuntu-22.04, 1.25.12, linux, arm64)
  • GitHub Check: Test macOS (1.25.12)
  • GitHub Check: Test macOS (1.26.5)
  • GitHub Check: Build and Test (ubuntu-22.04, 1.26.5, linux, arm64)
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
cmd/root.go (1)

471-471: LGTM!


📝 Walkthrough

Walkthrough

Logging initialization now passes the log-rotating writer directly to the CLI logger instead of its underlying file.

Changes

Logging integration

Layer / File(s) Summary
Pass the rotating writer to the CLI logger
cmd/root.go
initLogging passes logrotate directly to cli.New.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: parkervcp

Poem

A bunny watched the log files turn,
And saw the rotating writer return.
“Pass the writer whole,”
It whispered with soul,
While neat CLI messages burn.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix to log rotation and the use of the wrapper, which matches the main change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@parkervcp
parkervcp merged commit d15839c into pelican:main Aug 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants