properly handle stdout, stderr, and null #1029
Conversation
📝 WalkthroughWalkthroughThe pull request updates the logging configuration across the application. The logging setup in the command’s root file is modified to use a switch statement that selects the log output based on the configured value. Explicit handling for Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
osterman
dismissed stale reviews from aknysh and coderabbitai[bot]
via
19f84b4
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden (1)
78-78: Simplify example usage by removing unnecessary parameters.Based on previous feedback, the example usage should not include
<component>and-s <stack>parameters as they are not applicable to the editorconfig validation command.- atmos validate editorconfig <component> -s <stack> -- <native-flags> + atmos validate editorconfig -- <native-flags>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tests/fixtures/scenarios/valid-log-level/atmos.yaml(1 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/snapshots/TestCLICommands_Valid_Log_Level_in_Config_File.stderr.golden
🧰 Additional context used
📓 Learnings (2)
tests/snapshots/TestCLICommands_atmos_--help.stdout.golden (1)
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden (1)
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (6)
tests/snapshots/TestCLICommands_Valid_Log_Level_in_Environment_Variable.stderr.golden (1)
3-3: LGTM! Debug output correctly reflects stderr logging.The debug output accurately shows the updated log file configuration, which aligns with the PR's objective to properly handle stderr output.
tests/snapshots/TestCLICommands_atmos_--help.stdout.golden (1)
42-42: LGTM! Help text accurately reflects stderr as default.The help text properly documents the updated default value while maintaining clear information about all supported file descriptors.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden (1)
61-61: LGTM! Help text correctly shows stderr as default.The help text properly reflects the updated default value for the logs-file flag.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden (1)
61-61: Same issues as previous file.The help text has the same changes and considerations as the previous file.
Also applies to: 78-78
tests/fixtures/scenarios/valid-log-level/atmos.yaml (2)
3-3: Refined Log Output RedirectionChanging the default log file from
/dev/stdoutto/dev/stderraligns with the intended logging behavior to ensure that logs do not interfere with output parsing (especially in CI environments). This update properly meets the PR objective.
8-8: Wildcard Pattern for Included PathsThe inclusion of the wildcard
"**/*"underincluded_pathsappears intended to capture all relevant files. Please verify that this broad pattern does not inadvertently include files that should be excluded, particularly in larger codebases.If needed, I can suggest refined patterns or filters to better target desired files.
|
These changes were released in v1.160.2. |
* properly handle stdout, stderr, and null * update snapshots * fix test
what
/dev/stderr,/dev/stdout, and/dev/nullwith charmbracelet logger.Important
This is a partial fix, that ensures logs go to
stderrby defaultThere's still a problem with the presidence order in which flags, envs, and config processes the log settings
why
/dev/stdoutreferences
Summary by CodeRabbit