Skip to content

Conversation

@rustatian
Copy link
Member

@rustatian rustatian commented Oct 16, 2025

Reason for This PR

ref: roadrunner-server/roadrunner#2146

Description of Changes

  1. New option: skip_line_ending for logs configuration. When set, ignores any value in the line_ending option and uses no line endings.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • All commits in this PR are signed (git commit -s).
  • The reason for this PR is clearly provided (issue no. or explanation).
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • Any user-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.

Summary by CodeRabbit

  • New Features
    • Added a new configuration option to opt out of automatic line-ending insertion in log output; this is respected across all logger modes and encoder formats.
  • Chores
    • Default initialization no longer forces a line-ending when unspecified, so behavior is now determined by the new setting or explicit configuration.

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
@rustatian rustatian requested a review from Copilot October 16, 2025 06:53
@rustatian rustatian self-assigned this Oct 16, 2025
@rustatian rustatian added the enhancement New feature or request label Oct 16, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

A new exported boolean field SkipLineEnding was added to Config and is propagated into all Zap EncoderConfig instances (production, development, raw, default). InitDefault no longer auto-sets a default LineEnding when empty; line-ending behavior is coordinated via SkipLineEnding.

Changes

Cohort / File(s) Summary
Config field + encoder propagation
config.go
Added exported SkipLineEnding bool to Config. Propagated SkipLineEnding: cfg.SkipLineEnding into all Zap EncoderConfig instances across production, development, raw, and default paths (including BuildLogger), adjusted related encoder fields formatting. Removed automatic defaulting of LineEnding in InitDefault; line-ending policy is now coordinated with SkipLineEnding. Minor comment/wording tweaks.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Call as Caller
    participant Cfg as Config (with SkipLineEnding)
    participant Init as InitDefault
    participant BL as BuildLogger
    participant Enc as Zap EncoderConfig

    Note over Cfg,Init: Configuration initialization
    Call->>Cfg: provide config (may omit LineEnding)
    Cfg->>Init: InitDefault(cfg)
    alt LineEnding empty
        Init-->>Cfg: do not auto-set LineEnding (SkipLineEnding governs)
    else LineEnding present
        Init-->>Cfg: keep explicit LineEnding
    end

    Note over Cfg,BL: Logger build uses coordinated policy
    Call->>BL: BuildLogger(cfg)
    BL->>Enc: set EncoderConfig fields
    BL->>Enc: set SkipLineEnding = cfg.SkipLineEnding
    Enc-->>BL: encoder(s) created with SkipLineEnding applied
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny flag hops into the config tree,
"SkipLineEnding" whispers — quiet and free,
It travels to encoders, neat and bright,
No default endings forced in the night,
Logging hops on, aligned and merry! 🎩✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main feature introduced—adding a skip_line_ending option—and is concise and clear without extra noise or irrelevant details.
Description Check ✅ Passed The author has filled in all required template sections, including a clear reason with an issue reference, a concise description of changes, the license acceptance statement, and a completed PR checklist.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/skip-line-endings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d5b414 and 91de841.

📒 Files selected for processing (1)
  • config.go (6 hunks)
🔇 Additional comments (1)
config.go (1)

79-80: The comment accurately describes the field's purpose.

The field declaration and comment are clear and appropriate.


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 and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Introduce a new logs configuration option to disable line endings in logger output.

  • Add skip_line_ending option to Config
  • Adjust default initialization for line-ending behavior
  • Wire the new option into logger construction

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
config.go Adds SkipLineEnding to Config, updates InitDefault logic, and attempts to pass SkipLineEnding into zapcore.EncoderConfig
go.work.sum Tooling-generated dependency checksum updates (no functional code changes)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ef0020 and 4d5b414.

⛔ Files ignored due to path filters (1)
  • go.work.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • config.go (7 hunks)
🔇 Additional comments (2)
config.go (2)

79-80: LGTM! Field declaration is correct.

The new SkipLineEnding configuration field is properly defined with appropriate visibility, naming, and documentation.


123-123: LGTM! Consistent propagation across all logger modes.

The SkipLineEnding field is correctly propagated to all Zap EncoderConfig instances (production, development, raw, and default modes), ensuring consistent behavior across all logger configurations.

Also applies to: 146-146, 163-163, 181-181

Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
@rustatian rustatian merged commit e535179 into master Oct 16, 2025
7 checks passed
@rustatian rustatian deleted the feature/skip-line-endings branch October 16, 2025 14:52
@github-project-automation github-project-automation bot moved this to ✅ Done in Jira 😄 Oct 16, 2025
@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (1ef0020) to head (91de841).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@     Coverage Diff      @@
##   master   #76   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants