better guards for compile command logging #112
Conversation
WalkthroughThis pull request updates the configuration date in Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application/Executioner
participant Vars as AppVars::INSTANCE()
participant Log as Log Manager
App->>Vars: Check dump() condition
alt dump() is true
App->>Log: Instantiate log directories (cmd, out, err)
App->>Log: Call mk() to create directories
App->>Log: Execute deletion of empty directories (delEmpty)
else dump() is false
App-->>Log: Skip log directory operations
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (7)
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
🧹 Nitpick comments (1)
src/maiken/build/obj.cpp (1)
196-213: Consider keeping directory declarations consistent with bin.cpp.While the changes improve directory handling by moving creation inside the dump condition, the approach differs from
bin.cpp:
- Here: Directories are declared outside and created inside the dump condition
- In bin.cpp: Both declaration and creation are inside the dump condition
Also, the
1argument for parent directory creation is missing here but present inbin.cpp.Consider applying this diff to maintain consistency:
-mkn::kul::Dir cmdLogDir(".mkn/log/" + buildDir().name() + "/obj/cmd"); -mkn::kul::Dir outLogDir(".mkn/log/" + buildDir().name() + "/obj/out"); -mkn::kul::Dir errLogDir(".mkn/log/" + buildDir().name() + "/obj/err"); if (AppVars::INSTANCE().dump()) { - cmdLogDir.mk(); - outLogDir.mk(); - errLogDir.mk(); + mkn::kul::Dir cmdLogDir(".mkn/log/" + buildDir().name() + "/obj/cmd", 1); + mkn::kul::Dir outLogDir(".mkn/log/" + buildDir().name() + "/obj/out", 1); + mkn::kul::Dir errLogDir(".mkn/log/" + buildDir().name() + "/obj/err", 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mkn.yaml(1 hunks)src/maiken/build.cpp(1 hunks)src/maiken/build/bin.cpp(1 hunks)src/maiken/build/obj.cpp(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- mkn.yaml
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: Python 3.12
- GitHub Check: build
- GitHub Check: Python 3.11
- GitHub Check: build (macos-14)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (3)
src/maiken/build.cpp (1)
52-60: LGTM! Improved directory cleanup with proper guards.The changes correctly guard the directory cleanup logic with
AppVars::INSTANCE().dump()check, ensuring that cleanup only occurs when dumping is enabled. The lambda function provides a clean abstraction for the cleanup logic.src/maiken/build/bin.cpp (1)
95-97: LGTM! Improved directory creation with proper guards.The changes correctly move directory creation inside the
AppVars::INSTANCE().dump()block, ensuring directories are only created when dumping is enabled. The use of1as the second argument ensures parent directories are created if needed.src/maiken/build/obj.cpp (1)
244-252: LGTM! Clean directory cleanup implementation.The directory cleanup logic is well-implemented:
- Properly guarded by
AppVars::INSTANCE().dump()- Uses a lambda function for cleanup, consistent with
build.cpp- Correctly removes empty directories
Summary by CodeRabbit