add command to validate config #3
Conversation
dhth
commented
Oct 18, 2025
dhth
commented
- add command to validate config
- simplify config path flag usage
📝 WalkthroughWalkthroughConfiguration loading was moved out of the root command and into individual commands: compare-modules now reads config during its RunE using a local --config-path flag. The centralized PersistentPreRunE-based config flow and ErrCouldntReadConfigFile in root were removed. A new config validate subcommand was added that reads and validates a config file and exposes ErrConfigValidationFoundErrors. Error variables were reorganized. main.go error handling was changed from a negated if to a switch that explicitly matches ErrModulesNotInSync and ErrConfigValidationFoundErrors. Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal/cmd/compare_modules.go (2)
51-62: Consider using PreRunE instead of PersistentPreRunE.Since this command doesn't appear to have subcommands,
PreRunEwould be more semantically appropriate.PersistentPreRunEis intended for hooks that should run for both a command and all its subcommands.Apply this diff:
- PersistentPreRunE: func(_ *cobra.Command, _ []string) error { + PreRunE: func(_ *cobra.Command, _ []string) error { configBytes, err := os.ReadFile(configPath) if err != nil { return fmt.Errorf("%w: %w", ErrCouldntReadConfigFile, err) } config, err = getConfig(configBytes) if err != nil { return err } return nil },
51-62: Extract duplicated config loading logic into a helper function.The config loading pattern (reading the file + calling getConfig) is duplicated between this file and
internal/cmd/config_cmd.go(lines 56-61). Consider extracting this into a helper function.Add this helper function to
internal/cmd/config.go:func loadConfigFromPath(path string) (domain.Config, error) { configBytes, err := os.ReadFile(path) if err != nil { return domain.Config{}, fmt.Errorf("%w: %w", ErrCouldntReadConfigFile, err) } return getConfig(configBytes) }Then use it in the PreRunE:
- PersistentPreRunE: func(_ *cobra.Command, _ []string) error { - configBytes, err := os.ReadFile(configPath) - if err != nil { - return fmt.Errorf("%w: %w", ErrCouldntReadConfigFile, err) - } - config, err = getConfig(configBytes) - if err != nil { - return err - } - - return nil + PreRunE: func(_ *cobra.Command, _ []string) error { + var err error + config, err = loadConfigFromPath(configPath) + return err },And similarly in
internal/cmd/config_cmd.goline 56.internal/cmd/config_cmd.go (1)
48-81: Validation command implementation looks good.The command properly validates the config and provides clear feedback. The error handling pattern (printing the error then returning a sentinel) correctly integrates with the error handling in
main.go.Note: The config loading duplication with
compare_modules.gohas already been flagged in that file's review.Minor: Line 67 could use
Printlnfor consistency with standard output patterns:- fmt.Printf("Configuration is valid\n") + fmt.Println("Configuration is valid")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
internal/cmd/compare_modules.go(3 hunks)internal/cmd/config.go(1 hunks)internal/cmd/config_cmd.go(2 hunks)internal/cmd/root.go(2 hunks)main.go(1 hunks)
🔇 Additional comments (7)
internal/cmd/config.go (1)
11-14: LGTM!The error declarations are properly consolidated using a var block, following standard Go conventions.
main.go (1)
17-23: LGTM!The switch-based error handling allows specific error types to suppress duplicate error printing (since they're already printed by the command), while maintaining the non-zero exit code. This pattern works correctly with the validation command that prints its own error messages.
internal/cmd/compare_modules.go (2)
12-14: LGTM!The simplified signature and local config state make the command self-contained, removing coupling to the root command's lifecycle.
81-87: LGTM!The config-path flag is properly defined with appropriate defaults and description.
internal/cmd/root.go (1)
1-41: LGTM!The refactoring successfully moves config loading responsibility from the root command to individual commands, resulting in a cleaner architecture with better separation of concerns.
internal/cmd/config_cmd.go (2)
15-15: LGTM!The error sentinel is properly defined and integrates well with the error handling pattern in
main.go.
24-24: LGTM!The validation subcommand is properly registered with the config command.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/cmd/compare_modules.go (1)
81-87: Flag definition looks good.The
--config-pathflag follows standard Cobra patterns with a sensible default. Note that this flag is also defined inconfig_cmd.gofor the validate command—if you find yourself adding more commands that need config loading, consider extracting a helper function to reduce duplication.internal/cmd/config_cmd.go (1)
56-68: Consider writing validation errors to stderr.Line 63 writes the validation error to stdout using
fmt.Println. Standard practice is to write error messages to stderr so they can be distinguished from normal output.Apply this diff:
- fmt.Println(err.Error()) + fmt.Fprintln(os.Stderr, err.Error())Also note: The config-loading pattern (lines 56-59) is duplicated in
compare_modules.go(lines 52-55). If you add more commands that need config loading, consider extracting a helper likeloadConfigFromPath(path string) (domain.Config, error)to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
internal/cmd/compare_modules.go(3 hunks)internal/cmd/config_cmd.go(2 hunks)
🔇 Additional comments (4)
internal/cmd/compare_modules.go (1)
51-62: LGTM! Clean PreRunE implementation.The config loading logic is well-structured: reads the file, wraps errors appropriately with
ErrCouldntReadConfigFile, and delegates parsing togetConfig. UsingPreRunEfor setup is a good separation of concerns.internal/cmd/config_cmd.go (3)
5-7: LGTM!Standard library imports appropriate for the new validation functionality.
15-15: Good practice to define error sentinel.Defining
ErrConfigValidationFoundErrorsas a package-level sentinel allows main.go and other code to distinguish validation failures from other errors.
72-78: Flag definition looks good.The
--config-pathflag follows standard Cobra conventions with appropriate defaults.
