-
-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get atmos config and vendor from .yaml or .yml #736
Conversation
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe changes primarily focus on enhancing the handling of configuration files across several components. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
- internal/exec/vendor_utils.go (1 hunks)
- pkg/config/config.go (1 hunks)
- pkg/utils/file_utils.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
pkg/utils/file_utils.go (1)
Line range hint
170-175
: LGTM: Improved code formattingThe indentation change in the
IsSocket
function improves code readability while maintaining the existing functionality.pkg/config/config.go (1)
341-354
: LGTM! Changes align well with PR objectives.The modifications to the
processConfigFile
function effectively implement the PR's goal of supporting bothatmos.yaml
andatmos.yml
files. The use ofu.SearchConfigFile(path)
likely allows for checking both file extensions, and the subsequent use ofconfigPath
ensures consistency throughout the function.These changes are minimal and focused, which reduces the risk of introducing new bugs while achieving the desired functionality enhancement.
internal/exec/vendor_utils.go (1)
135-137
: Enhanced configuration file search improves flexibilityThe use of
u.SearchConfigFile(foundVendorConfigFile)
enhances the flexibility by allowing the system to search for configuration files with different extensions, aligning with the PR objectives to support bothatmos.yaml
andatmos.yml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range comments (2)
internal/exec/vendor_utils.go (2)
Line range hint
131-141
: Consider refactoring to unify search pathsCurrently, after calling
u.SearchConfigFile(vendorConfigFile)
, the code checkscliConfig.BasePath
separately if the file is not found. To streamline the search process and reduce redundancy, consider updatingu.SearchConfigFile
to includecliConfig.BasePath
in its search paths. This would centralize the file-search logic and potentially simplify error handling.
Line range hint
137-139
: Enhance error message for missing configuration fileWhen the vendor configuration file is not found, the error message only references the path constructed with
cliConfig.BasePath
:fmt.Errorf("vendor config file '%s' does not exist", pathToVendorConfig)To provide clearer guidance to the user, consider updating the error message to include all the locations searched for the configuration file. This will help users understand where the file is expected and troubleshoot more effectively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- cmd/cmd_utils.go (2 hunks)
- internal/exec/vendor_utils.go (1 hunks)
- pkg/config/const.go (1 hunks)
- pkg/utils/file_utils.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
internal/exec/vendor_utils.go (1)
Line range hint
128-144
: Configuration file search logic correctly updatedThe
ReadAndProcessVendorConfigFile
function now utilizesu.SearchConfigFile(vendorConfigFile)
to enhance the search for vendor configuration files, allowing it to locate files with both.yaml
and.yml
extensions as intended in the PR objectives. The additional check incliConfig.BasePath
ensures that the configuration file is found even when it's not in the current directory, maintaining compatibility with existing configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @haitham911
These changes were released in v1.94.0. |
what
why
references
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores