Rename pipeline builder shortcut to RunAsync - #4252
Conversation
Align the one-call builder path with IPipeline and host naming while preserving module ExecuteAsync terminology. BREAKING CHANGE: PipelineBuilder.ExecutePipelineAsync is removed. Use PipelineBuilder.RunAsync. Refs #4222
|
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 (86)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe public pipeline builder method was renamed from ChangesPipeline API and documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change renames the public pipeline builder shortcut while preserving pipeline execution behavior; consumers must update source references, but no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The implementation renames PipelineBuilder.ExecutePipelineAsync() to RunAsync(), removes the former call sites, and updates active C#, F#, template, test, README, and documentation examples. Module ExecuteAsync() usage remains unchanged, matching issue Full details: Docstring CoverageExplanation Docstring coverage is 1.01% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 199 functions across 50 files. (36 skipped: 23 unsupported, 13 over the file limit.)
✨ 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 renames the one-call pipeline-builder shortcut from
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/ModularPipelines/Extensions/PipelineBuilderExtensions.cs | Renames the public builder convenience extension to RunAsync without changing its build-run-dispose implementation or cancellation forwarding. |
| docs/docs/examples/fsharp-interactive.md | Current HEAD preserves ExecutePipelineAsync for the example’s explicitly referenced V3 packages, resolving the previous compatibility finding. |
| test/ModularPipelines.UnitTests/Api/RootNamespaceGoldenPathCompileFixture.cs | Updates the root-namespace compile fixture to exercise the renamed RunAsync extension. |
| test/ModularPipelines.FSharp.TestFixtures/PipelineModules.fs | Migrates active F# fixtures to the current RunAsync builder API. |
Reviews (2): Last reviewed commit: "docs: keep v3 F# example compatible" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
GitHub Actions
Inactive
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: #4252 — Rename ExecutePipelineAsync → RunAsync
This is a clean, mechanical rename with balanced diff stats (290/290) across 87 files — no logic changes. Verified that PipelineBuilderExtensions.RunAsync(this PipelineBuilder ...) calls IPipeline.RunAsync(cancellationToken), a distinct member on a distinct type, so there's no recursive/naming collision. Call sites (C#, F# fixtures, templates, doc snippets) were updated consistently and remaining ExecutePipelineAsync references are confined to the intentionally-preserved v3 history docs (RELEASE_NOTES_V3.md, docs/docs/migrating-to-v3.md).
Actionable finding
docs/docs/examples/fsharp-interactive.md (line 62) pins the script to the v3 NuGet package (#r "nuget: ModularPipelines.DotNet, 3.*" at line 25, with an explicit comment "retain compatibility with the v3 package above" at line 43) but was migrated to call builder.RunAsync(). RunAsync() doesn't exist on v3's PipelineBuilder — that name is introduced by this very PR — so the example now instructs readers to write code that won't compile against the pinned v3 package. This is the one doc in the migrated set that explicitly targets an old package version; every other touched doc (getting-started.md, fundamentals.md, categories.md, single-file-csharp.md) implicitly targets the current package and is fine as RunAsync().
Fix: either revert this specific call site back to builder.ExecutePipelineAsync() (consistent with the v3 package pin, mirroring how the other v3-history docs were left alone), or bump the pinned version in this example to the current major and keep RunAsync(). The former is simpler and matches the PR's stated intent of leaving v3-targeted content alone.
No other correctness, architectural, or maintainability concerns found — this is a pure rename otherwise.
Use the API available in the documented 3.x package. Refs #4222.
There was a problem hiding this comment.
Review: #4252 — Rename ExecutePipelineAsync → RunAsync
Re-reviewed at head 62a40b8566f5577b1a31278e90de45ad38e7d3c0. This is a clean, purely mechanical rename with no logic changes:
PipelineBuilderExtensions.RunAsync(this PipelineBuilder ...)insrc/ModularPipelines/Extensions/PipelineBuilderExtensions.csdelegates toIPipeline.RunAsync(cancellationToken)— a distinct member on a distinct type, so there's no recursive call or naming collision.- Every other changed line across the ~88 touched files (docs, README, templates, XML doc comments, C#/F# call sites) is a 1:1 token replacement of
ExecutePipelineAsync→RunAsync. - Historical v3 docs (
RELEASE_NOTES_V3.md,docs/docs/migrating-to-v3.md,docs/versioned_docs/version-3.x/**) were correctly left untouched, matching the PR's stated goal of preserving historical migration guidance.
Previous finding — now resolved
My prior review (at commit e6dd02164acaf89792f9332b28b24c7c360eb85c) flagged that docs/docs/examples/fsharp-interactive.md pins the script to the v3 NuGet package but had been migrated to call builder.RunAsync(), which doesn't exist on v3's PipelineBuilder. The follow-up commit "docs: keep v3 F# example compatible" reverts that call site back to builder.ExecutePipelineAsync() (confirmed at line 62 of the current head), consistent with the nuget: ModularPipelines.DotNet, 3.* pin in the same file. This is exactly the fix I suggested.
No other correctness, architectural, or maintainability concerns. The RunAsync extension naming is fine since it lives on PipelineBuilder, distinct from the module-level ExecuteAsync() convention, so there's no ambiguity for readers.
GitHub Actions
Inactive
* build(api): baseline shipped package surfaces Make every public API addition or removal explicit through RS0016 and RS0017 across all release packages. Refs #4249 * fix(ci): enforce integration API baselines Run API analyzers in ordinary CI tool builds. Refs #4249. * fix(api): cover all package baselines Baseline every package-shaped integration and fail CI when a package lacks PublicApiAnalyzers files.\n\nRefs #4249 * fix(api): refresh DotNet baseline Record the generated API merged by #4246 so the CI public API gate passes. Refs #4249. * fix(api): refresh rebased baselines Record public APIs added by #4246 and keep ordinary CI baseline checks lean. Refs #4249. * fix(api): refresh merged core baseline Record API changes merged by #4252 and #4253. Refs #4249. * fix(api): baseline repeat option terminator * fix(api): complete CI baselines Record declarations missed by memory-limited local captures, exempt generated-option verification until compatibility checks run, and cover the baseline assertion script. * fix(api): refresh context baseline Track the context API consolidation merged in #4248 after rebasing the public API gate. * fix(api): baseline Azure tool accessors * fix(api): refresh merged core surface * fix(ci): baseline generated API changes * fix(ci): restore API baseline updates Refresh the linked OptionsGenerator baseline, restore PublicApiAnalyzers before applying code fixes, and allow only manifest-authorized baseline files to exceed the generated-file size cap. * fix(ci): isolate public API analyzers * fix(api): refresh merged core baseline * fix(api): refresh cmd baseline * fix(ci): gate public API analyzers Keep ordinary local builds lean while enforcing API baselines in GitHub Actions and explicit analyzer runs. * fix(api): refresh merged core baseline Record main's latest v4 surface changes so CI analyzer enforcement remains exact after rebase. * fix(ci): restore tool analyzer coverage * fix(api): refresh merged public surfaces
1 participant
Summary
PipelineBuilder.ExecutePipelineAsync()extension toRunAsync()Breaking change
PipelineBuilder.ExecutePipelineAsync()is removed. CallPipelineBuilder.RunAsync()instead. Module implementations continue to useExecuteAsync().Validation
RunAsync()existedRunAsync_ValidatesBeforeRunningtest (1 passed)DocumentationSnippetTests(4 passed)ModularPipelines.slnxnpm run build --prefix docsExecutePipelineAsyncreferences outside preserved v3 historygit diff --checkdotnet format whitespace --verify-no-changesreports pre-existing formatting debt in unchanged portions ofScaleTests.cs; this token-only migration introduces no whitespace diff.Closes #4222
Summary by CodeRabbit
Breaking Changes
ExecutePipelineAsync()toRunAsync().RunAsync()when starting pipelines.Documentation
Tests
RunAsync()while preserving existing scenarios and validations.