env var doubling fix #104
env var doubling fix #104
Conversation
WalkthroughThe pull request introduces several modifications across multiple files. The Changes
Possibly related PRs
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 (
|
![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
🧹 Outside diff range and nitpick comments (1)
src/maiken/setup.cpp (1)
43-51: Approve changes with performance considerations.The modifications to the
sub_initializerfunction introduce a more granular approach to processing theSTR_SUBnode content. This change appears to address potential issues with environment variable expansion by applyingProperties::RESOLVEbefore splitting the arguments.Consider the following suggestions:
For large inputs, the nested loop structure might impact performance. If performance becomes an issue, consider refactoring to reduce the nesting depth.
To improve readability and maintainability, you could extract the inner loop logic into a separate function:
void processSubLine(const Application& app, const std::string& line) { for (auto const& resolved_line : mkn::kul::cli::asArgs(Properties::RESOLVE(app, line))) { auto pInfo = ProjectInfo::PARSE_LINE(resolved_line); mkn::kul::Dir local{pInfo.local}; if (!local) SCMGetter::GET(local, pInfo.scm) ->co(local.path(), SCMGetter::REPO(local, pInfo.scm), pInfo.version); } } // In the main function: for (auto const& input_line : lines) { processSubLine(app, input_line); }This refactoring would improve the function's readability without changing its behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- mkn.yaml (1 hunks)
- src/maiken/setup.cpp (2 hunks)
- src/settings.cpp (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- mkn.yaml
🔇 Additional comments (3)
src/settings.cpp (2)
152-177: LGTM: Improved code readabilityThe changes to the
validator()method have improved the code's readability by reformatting the return statement. The structure and content of theValidatorobject remain unchanged, preserving the existing functionality.
Line range hint
1-1: Verify environment variable handling changesThe AI summary mentions the removal of references to
appVars.envVar(ev.name(), ev.toString());in thePOST_CONSTRUCTmethod. However, these changes are not visible in the provided code segment. Please verify:
- The impact of removing
appVars.envVar(ev.name(), ev.toString());on environment variable handling.- Whether this change affects any other parts of the codebase that might have depended on
appVars.envVar().- If there are any necessary updates to documentation or tests related to environment variable handling.
To help verify these changes, you can run the following script:
✅ Verification successful
Environment Variable Handling Successfully Updated
The removal of
appVars.envVar(ev.name(), ev.toString());has been successfully completed. All environment variable handling in the codebase now utilizesmkn::kul::env::SET, as confirmed by the search results.
mkn::kul::env::SETis consistently used insrc/settings.cppandsrc/maiken/process.cpp.- No remaining references to
appVars.envVarwere found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of appVars.envVar rg "appVars\.envVar" --type cpp # Search for new environment variable handling methods rg "mkn::kul::env::SET" --type cppLength of output: 417
src/maiken/setup.cpp (1)
32-33: Verify the intention behind reordering include statements.The order of include statements has been changed, with "maiken/regex.hpp" now preceding "maiken/github.hpp". While this change doesn't affect functionality, it's important to maintain consistency across the codebase.
Could you clarify if this reordering was intentional and if there's a specific reason for it?
Summary by CodeRabbit
DATEproperty in the configuration to reflect the new date.STR_SUBnode, allowing for more granular handling of input lines.validator()method return statement and streamlined environment variable handling.