Skip to content

Fix WORKFLOW.md D4.5 Wording Contradicting Its Own Input and D1.3 - #898

Merged
ptr727 merged 1 commit into
developfrom
fix-workflow-d4-5-wording
Aug 21, 2026
Merged

Fix WORKFLOW.md D4.5 Wording Contradicting Its Own Input and D1.3#898
ptr727 merged 1 commit into
developfrom
fix-workflow-d4-5-wording

Conversation

@ptr727

Copy link
Copy Markdown
Owner

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 stated its input as a real publish, then ended on a clause about
smoke ('still lets docker build on smoke'), contradicting the
guarantee's own input and D1.3 (smoke never publishes or pushes).
Verified against the current build-release-task.yml / build-docker-task.yml:
build-docker's if: (enable_docker && !failure() && !cancelled()) tolerates a
skipped upstream target but not a failed one, and whether Docker actually
pushes is the separate push: (dockerhub && !smoke) input forwarded into
build-docker-task.yml's build-push-action call. A disabled/unchanged
(skipped) target therefore still lets Docker push on a real, non-smoke
publish, matching the corrected wording.
Fixes#745
CopilotAI lite review requested due to automatic review settings August 21, 2026 21:00
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clarify WORKFLOW.md D4.5 wording about Docker push vs smoke builds

📝 Documentation🕐 Less than 10 minutes

Grey Divider

AI Description

• Correct D4.5 wording to match the workflow’s actual Docker push gating behavior.
• Remove the contradictory smoke reference and state that skipped targets still allow Docker push.
Diagram

graph TD
A["WORKFLOW.md"] --> B["build-release-task.yml"] --> C["build-docker-task.yml"] --> D{{"Docker Hub"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add a doc-to-workflow consistency check
  • ➕ Catches future doc drift automatically (CI-visible).
  • ➕ Encodes the intended semantics (e.g., smoke implies no push) as assertions.
  • ➖ Additional maintenance burden and may be brittle as workflow logic evolves.
  • ➖ Hard to fully validate prose statements without overfitting checks.
2. Move the nuance into workflow comments only
  • ➕ Keeps implementation-adjacent explanation next to the if:/push: expressions.
  • ➕ Reduces risk of separate long-form docs drifting.
  • ➖ Makes the high-level behavioral contract less self-contained.
  • ➖ Comments are easier to miss than a dedicated contract section.

Recommendation: Proceed with this PR’s documentation fix as the right minimal correction. If doc drift becomes recurrent, consider adding a lightweight CI check (or a short workflow comment pointing back to D4.5) rather than trying to fully mechanize prose validation.

Files changed (1) +1 / -1

Documentation (1) +1 / -1
WORKFLOW.mdFix D4.5 wording to describe Docker push behavior correctly+1/-1

Fix D4.5 wording to describe Docker push behavior correctly

• Updates guarantee D4.5 to remove the contradictory reference to smoke builds and correctly state that skipped (disabled/unchanged) targets can still allow Docker push on a real publish run.

WORKFLOW.md

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a narrow prose correction that removes a contract contradiction without altering any implementation behavior.

Pull request overview

Clarifies the WORKFLOW.md D4.5 publish guarantee by removing wording that incorrectly referenced smoke behavior, aligning the guarantee’s stated input (“a real publish”) with the documented Docker push mechanism.

Changes:

  • Reword D4.5’s final clause from “docker build on smoke” to “docker push” to avoid contradicting D1.3 and D4.5’s own stated input.
  • Keeps the statement focused on the skipped-vs-failed behavior (!failure() && !cancelled()) without implying smoke publishes.
File summaries
FileDescription
WORKFLOW.mdFixes D4.5 wording to match the real-publish contract and the Docker push mechanism.
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.

@coderabbitai

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6c10753b-14a3-452c-a37b-a4eb754aa703

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@ptr727
ptr727 merged commit c5fec4a into developAug 21, 2026
9 checks passed
@ptr727
ptr727 deleted the fix-workflow-d4-5-wording branch August 21, 2026 21:05
@ptr727ptr727 mentioned this pull request Aug 21, 2026
ptr727 added a commit that referenced this pull request Aug 21, 2026
## Summary
- Fix `WORKFLOW.md` D4.5 wording that contradicted its own stated input
and D1.3.
## Included Work
- `c5fec4a` Fix WORKFLOW.md D4.5 Wording Contradicting Its Own Input and
D1.3 (#898).
## Tracking
Fixes#745.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Clarified release workflow behavior when enabled builds fail.
* Documented that skipped disabled or unchanged targets still allow
Docker publication to proceed.
* Removed the incorrect statement that Docker builds occur during smoke
runs.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727