docs: clarify Nuke-only publish for the FFmpeg worker; mark matrix collapse done#2015
Conversation
…llapse done - Beutl.csproj: document that CopyFFmpegWorkerForApp runs AfterTargets="Build" only, so a bare `dotnet publish` (NukePublish!=true) deliberately omits the GPL worker. Publishing a runnable app must go through Nuke (NukePublish=true); non-Nuke publish is a build-graph convenience, not a supported distribution path. - refactoring-plan.md: the deferred FFmpegOutOfProcess csproj-matrix collapse has landed (no FFmpegOutOfProcess/FFMPEG_OUT_OF_PROCESS references remain in src/), so the out-of-process/worker path is unconditional. Mark the item done. Refs: Project #9 "AI Review" items 155, 164
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated the FFmpeg refactoring plan to mark the media-backend cleanup complete and clarified the Beutl project comments for FFmpeg worker copy behavior. ChangesFFmpeg cleanup notes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates documentation to clarify the supported publishing path for the GPL Beutl.FFmpegWorker process (Nuke-only) and marks a previously tracked FFmpeg build-matrix follow-up as completed in the refactoring plan.
Changes:
- Expand the
Beutl.csprojMSBuild comment nearCopyFFmpegWorkerForAppto explain that plaindotnet publishoutputs intentionally omit the worker and that Nuke publish is the supported distribution path. - Update
docs/refactoring-plan.mdto mark the FFmpeg out-of-process matrix collapse follow-up as Done.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Beutl/Beutl.csproj | Clarifies (in an XML comment) the intended dev vs release/publish behavior for the FFmpeg worker. |
| docs/refactoring-plan.md | Marks the FFmpeg build-matrix collapse follow-up as completed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Review follow-up (#2015). The prior note claimed CopyFFmpegWorkerForApp "does NOT run on Publish", but `dotnet publish` runs the Build target, so an AfterTargets="Build" target still runs during the publish build step (unless --no-build). The accurate point is that the target copies the worker into the build output ($(OutDir), i.e. bin), not $(PublishDir), so a bare `dotnet publish` leaves the worker out of the published output. Reword the comment accordingly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65418d737b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nstraint Keep the footgun (a bare `dotnet publish` with NukePublish!=True omits the worker and yields a non-runnable app; a runnable publish must go through Nuke), and drop the dev-build narration and editorializing.
|
No TODO comments were found. |
Minimum allowed line rate is |
Description
Documents the intended publish path for the GPL
Beutl.FFmpegWorkerprocess and closes two stale board follow-ups as documentation.src/Beutl/Beutl.csproj: a comment nearCopyFFmpegWorkerForApp(AfterTargets="Build") clarifies that a baredotnet publishdoes not carry the worker — publishing must go through Nuke (NukePublish=true), which is the supported/release path. No build behavior changes.docs/refactoring-plan.md: marks the FFmpegOutOfProcess build-matrix collapse follow-up as Done.Per the maintainer decision, "include the worker in non-Nuke publish outputs" is resolved as documented Nuke-only rather than wiring the worker into bare
dotnet publish(which would risk dragging GPL output into MIT publish flows).Affected areas
Breaking changes
None — comment + docs only.
Test plan
No code paths changed. Verified that the existing
CopyFFmpegWorkerForApptarget is build-order only and that the Nuke publish target remains the worker-carrying path; full-solution build stays green.Fixed issues / References
Summary by CodeRabbit