Update editorconfig to version 3.1.2 #955
Conversation
📝 WalkthroughWalkthroughThe pull request introduces enhancements to the EditorConfig validation command in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (5)
Finishing Touches
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/validate_editorconfig.go (1)
156-160: Clean error handling, but let's add some battle logs! 🗡️The simplified error handling is good, but consider adding debug logging before exit.
er.PrintErrors(errors, config) u.LogDebug(atmosConfig, fmt.Sprintf("%d files checked", len(filePaths))) errorCount := er.GetErrorCount(errors) if errorCount != 0 { + u.LogDebug(atmosConfig, fmt.Sprintf("Validation failed with %d errors", errorCount)) os.Exit(1) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/validate_editorconfig.go(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (22)
- GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
- GitHub Check: [mock-macos] examples/demo-vendoring
- GitHub Check: [mock-macos] examples/demo-context
- GitHub Check: [mock-macos] examples/demo-component-versions
- GitHub Check: [mock-macos] examples/demo-atlantis
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: [mock-windows] examples/demo-context
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: [mock-windows] examples/demo-atlantis
- GitHub Check: [mock-linux] tests/fixtures/scenarios/complete
- GitHub Check: [mock-linux] examples/demo-vendoring
- GitHub Check: [mock-linux] examples/demo-context
- GitHub Check: [mock-linux] examples/demo-component-versions
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: [mock-linux] examples/demo-atlantis
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: Docker Lint
- GitHub Check: Summary
🔇 Additional comments (3)
cmd/validate_editorconfig.go (3)
22-29: Well structured variable declarations, warrior! 💪The introduction of
defaultConfigFileNamesslice with multiple config file options (.editorconfig, .editorconfig-checker.json, .ecrc) provides excellent flexibility for users.
15-17: Verify editorconfig-checker version alignment! 🎯The imports are correctly using v3, but let's verify the exact version in go.mod.
✅ Verification successful
Version alignment confirmed! 💪
The imports correctly use v3 path and align perfectly with go.mod's v3.1.2 specification. Keep marching forward!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify editorconfig-checker version grep -r "github.com/editorconfig-checker/editorconfig-checker/v3" go.modLength of output: 136
49-56: Strong configuration initialization approach! 🛡️The new configuration path handling is more robust, supporting multiple config file locations. However, let's verify the behavior when multiple config files exist.
✅ Verification successful
Configuration handling is battle-tested! 💪
The implementation correctly handles multiple config files with a clear priority order. When multiple config files exist, they are processed hierarchically:
- System directory configs are loaded first
- Home directory configs are merged next
- Current directory configs take precedence
- Environment variables and command-line arguments get highest priority
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the code handles multiple config files correctly ast-grep --pattern 'NewConfig($paths)' | rg -A 5 'NewConfig'Length of output: 60
Script:
#!/bin/bash # Find config-related files fd "config.*\.go$" # Search for NewConfig function implementation rg -l "func NewConfig" # Look for multiple config handling logic rg -A 5 "type Config struct"Length of output: 827
Script:
#!/bin/bash # Examine the config package implementation cat pkg/config/config.go # Look for specific config loading patterns rg -A 10 "func NewConfig" pkg/config/config.go # Check for any config merging or priority logic rg -A 5 "load.*config" pkg/config/config.goLength of output: 13658
aknysh
added
no-release
samtholiya
dismissed stale reviews from aknysh and coderabbitai[bot]
via
8874ad5
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/test-cases/help-and-usage.yaml (3)
434-446: Add stdout/stderr expectations for consistency.The test case should include stdout/stderr expectations like other help command tests in this file. This ensures we validate both the exit code and the expected output.
expect: + stdout: + - "Usage:" + - "Flags:" + stderr: + - "^$" exit_code: 0
460-472: Add stderr expectation for error message.The test case should verify the error message for the non-existent subcommand, following the pattern of other similar tests in this file.
expect: + stderr: + - 'Error: Unknown command \"non-existant\" for \"atmos validate editorconfig\"' exit_code: 1
473-473: Remove extra blank lines at the end of file.The file has too many blank lines at the end. Keep only one blank line.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 473-473: too many blank lines
(1 > 0) (empty-lines)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
cmd/validate_editorconfig.go(7 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_non-existant.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_non-existant.stdout.golden(1 hunks)tests/test-cases/help-and-usage.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/snapshots/TestCLICommands_atmos_validate_editorconfig_non-existant.stdout.golden
🧰 Additional context used
🪛 yamllint (1.35.1)
tests/test-cases/help-and-usage.yaml
[warning] 473-473: too many blank lines
(1 > 0) (empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (7)
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_non-existant.stderr.golden (1)
1-2: Error message looks good!The error message is clear, concise, and follows standard CLI error message format.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden (1)
1-73: Help text structure looks good!The help text is well-organized, comprehensive, and follows standard CLI help format.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden (1)
1-78: Skipping review of duplicate help text.This file contains identical content to
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden.cmd/validate_editorconfig.go (3)
22-29: Good enhancement to configuration file support!The addition of multiple default config file names (
defaultConfigFileNames) improves flexibility by supporting various common EditorConfig file names.
94-104: Format validation needs refactoring.The format validation logic is duplicated and contains an incorrect error message in the second validation block.
This is a duplicate of a previous review comment. The suggested refactoring to extract the validation logic and fix the error message is still valid.
Line range hint
139-164: Streamlined error handling looks good!The error handling has been simplified:
- Debug logging of config and file paths
- Clear error count tracking
- Simple exit with status code 1 on errors
tests/test-cases/help-and-usage.yaml (1)
447-459: Similar stdout/stderr expectations needed here.Same feedback as the previous test case applies here.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden
Outdated
Show resolved
Hide resolved
aknysh
deleted the
feature/dev-2952-update-editorconfig-library-version-to-312
branch
|
These changes were released in v1.152.1. |
what
why
Why snapshots changed
The new update consolidates the errors
Old error display strategy accoding to
v3.0.3:In the above image the lines were shown as two different errors
New error display strategy according to
v3.1.2Now the error line is single mentioning the error in both the lines
references
Summary by CodeRabbit
New Features
atmos validate editorconfigcommand, including flags and options.Bug Fixes
Chores