Flatten shell command execution API - #4255
Conversation
Expose the common command primitive directly on IShellContext and remove the redundant generic options wrapper.\n\nRefs #4213
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (33)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR replaces nested shell command execution with ChangesShell execution API migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change simplifies the shell command API while preserving existing execution and cancellation controls. No actionable merge-blocking risk remains beyond normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation Docstring coverage is 19.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 78 functions across 30 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThe PR flattens general command execution onto
Confidence Score: 5/5The PR appears safe to merge, with the flattened API preserving the existing command execution lifecycle and the public surface removals explicitly forming part of the intended migration. The new shell overloads delegate to the same injected command context and correctly preserve arguments, execution options, cancellation, interception, logging, and secret handling; no actionable defect remains.
|
| Filename | Overview |
|---|---|
| src/ModularPipelines/Context/Domains/IShellContext.cs | Replaces the nested command property with direct options-based and raw-command execution overloads. |
| src/ModularPipelines/Context/Domains/Implementations/ShellContext.cs | Implements the flattened API by forwarding execution options and cancellation to the existing command service. |
| src/ModularPipelines/Options/CommandLineToolOptions.cs | Makes the options base concrete and adds direct runtime-tool construction while retaining protected construction for typed derived options. |
| src/ModularPipelines/Options/GenericCommandLineToolOptions.cs | Removes the now-redundant generic options wrapper as part of the intentional API migration. |
| src/ModularPipelines.Git/GitCommandRunner.cs | Migrates raw Git execution to the flattened shell API without changing execution settings or cancellation. |
| src/ModularPipelines.Node/Node.cs | Migrates the Node version command to the raw tool-and-arguments overload. |
| src/ModularPipelines.Node/Nvm.cs | Migrates NVM operations to the raw shell overload while preserving command arguments and cancellation. |
| test/ModularPipelines.UnitTests/Context/ShellContextTests.cs | Verifies delegation, raw-options construction, cancellation-token overload behavior, and removal of command nesting. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
M[Module or integration] --> S[IShellContext.RunAsync]
S --> O[CommandLineToolOptions]
O --> C[ICommandContext]
C --> I[Interceptors and secret handling]
C --> P[Process execution]
P --> R[CommandResult]
Reviews (1): Last reviewed commit: "feat(shell): flatten command execution" | Re-trigger Greptile
GitHub Actions
Failure
GitHub Actions
Inactive
1 participant
Summary
IShellContext.RunAsyncCommandLineToolOptions(string tool)concrete and removeGenericCommandLineToolOptionsValidation
ModularPipelines.Tests.slnfRelease buildCloses #4213
Summary by CodeRabbit
New Features
Documentation
Tests