Conversation
D4.5 states its input as **a real publish**, then ended on a clause
about smoke ("still lets docker build on smoke"), contradicting the
guarantee's own stated input and D1.3 (smoke publishes and uploads
nothing). Corrected to "still lets docker push", which describes the
actual mechanism with no reference to smoke.
Verified against the current `build-release-task.yml` /
`build-docker-task.yml`, not just the issue's own reasoning, since the
issue was filed a while ago:
- `build-docker`'s `if:` is `inputs.enable_docker && !failure() &&
!cancelled()`, needing `dotnet-publish`/`build-nuget`/`build-pypi`. A
**skipped** (disabled/unchanged) upstream target doesn't trip
`failure()`, so the job still runs; a **failed** one does, so it's
skipped.
- Whether Docker actually pushes is the separate `push: ${{
inputs.dockerhub && !inputs.smoke }}` input, forwarded straight into
`build-docker-task.yml`'s `docker/build-push-action` call, independent
of which upstream targets were skipped.
- Cross-checked against D1.3 and the other `WORKFLOW.md` passages
referencing this mechanism (D1 aggregator note, Docker-image section) —
none restate the flawed wording, all agree with the fix.
Relates to #745. `Fixes#745` lands on the follow-up `develop -> main`
promotion PR per the fleet's issue-closing convention (auto-close only
fires on a merge to the default branch).D4.5 states its input as **a real publish**, then ended on a clause
about smoke ("still lets docker build on smoke"), contradicting the
guarantee's own stated input and D1.3 (smoke publishes and uploads
nothing). Corrected to "still lets docker push", which describes the
actual mechanism with no reference to smoke.
Verified against the current `build-release-task.yml` /
`build-docker-task.yml`, not just the issue's own reasoning, since the
issue was filed a while ago:
- `build-docker`'s `if:` is `inputs.enable_docker && !failure() &&
!cancelled()`, needing `dotnet-publish`/`build-nuget`/`build-pypi`. A
**skipped** (disabled/unchanged) upstream target doesn't trip
`failure()`, so the job still runs; a **failed** one does, so it's
skipped.
- Whether Docker actually pushes is the separate `push: ${{
inputs.dockerhub && !inputs.smoke }}` input, forwarded straight into
`build-docker-task.yml`'s `docker/build-push-action` call, independent
of which upstream targets were skipped.
- Cross-checked against D1.3 and the other `WORKFLOW.md` passages
referencing this mechanism (D1 aggregator note, Docker-image section) —
none restate the flawed wording, all agree with the fix.
Relates to #745. `Fixes#745` lands on the follow-up `develop -> main`
promotion PR per the fleet's issue-closing convention (auto-close only
fires on a merge to the default branch).Code Review by Qodo🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships |
PR Summary by QodoFix WORKFLOW.md D4.5 wording about Docker behavior on skipped builds
AI Description
High-Level Assessment
Files changed (1) |
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughWORKFLOW.md updates D4.5 to state that failed builds block Docker publication and GitHub release creation. Skipped disabled or unchanged targets do not block Docker publication. The text no longer states that Docker builds during smoke runs. ChangesPublication contract
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk:⚪ Minimal · up to This PR makes a localized documentation wording correction and introduces no actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟢 Approval recommended
The change is a small, doc-only correction that removes a contradiction without altering any executable behavior.
Pull request overview
This PR promotes the develop state to main by applying the already-verified documentation fix for WORKFLOW.md D4.5, aligning the guarantee’s wording with its stated input (a real publish) and the workflow mechanics.
Changes:
- Update D4.5 wording to remove the incorrect “on smoke” clause and correctly describe the skipped-vs-failed behavior as “still lets docker push”.
File summaries
| File | Description |
|---|---|
| WORKFLOW.md | Corrects the D4.5 contract wording to match the publish/run conditions and Docker push gating behavior. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Summary
WORKFLOW.mdD4.5 wording that contradicted its own stated input and D1.3.Included Work
c5fec4aFix WORKFLOW.md D4.5 Wording Contradicting Its Own Input and D1.3 (Fix WORKFLOW.md D4.5 Wording Contradicting Its Own Input and D1.3 #898).Tracking
Fixes#745.
Summary by CodeRabbit