.github/workflows/create-release.yml:121-124 carries the same over-generalisation that #958 is removing from the prose. Found by the cross-review on #959.
The comment says:
tag, pypi and conda below do the same for each of their own direct dependencies, but spelled as !cancelled() && (needs.<job>.result == 'success' || needs.<job>.result == 'skipped') rather than the sibling files' != 'failure'
"each of their own direct dependencies" is false for version_check, which is a direct dependency of all three — needs: [ github, version_check ] at :443 and :521, [ tag, github, version_check ] at :620 — and is required to have strictly succeeded, with no || 'skipped':
:445 !cancelled() && needs.version_check.result == 'success' && ...
:523 !cancelled() && needs.version_check.result == 'success' && ...
:622 !cancelled() && needs.version_check.result == 'success' && ...
The equality pair applies only to the publishing predecessors: github, plus tag for conda.
Why it is worth fixing rather than ignoring. This comment is where the documentation got the claim from — releasing.rst said the same thing, and #959 corrects it there. Once #959 merges, the docs will describe this workflow more accurately than the workflow describes itself, and the next person to reconcile the two will reasonably trust the code comment. The comment is also load-bearing in its own right: it is the record of why this spelling was chosen over != 'failure', and that reasoning is correct — only the scope of the generalisation is wrong.
Fix is one clause: name the publishing predecessors and say version_check is required to have strictly succeeded. No behaviour change.
Note tests/project/test_release_gates.py pins the conditions but not this: there is no version_check.result assertion in the file, so neither the comment nor the prose had test cover for that clause. Adding one would be a reasonable part of the same change.
This touches .github/, so it needs a pull request rather than a direct push.
.github/workflows/create-release.yml:121-124carries the same over-generalisation that #958 is removing from the prose. Found by the cross-review on #959.The comment says:
"each of their own direct dependencies" is false for
version_check, which is a direct dependency of all three —needs: [ github, version_check ]at:443and:521,[ tag, github, version_check ]at:620— and is required to have strictly succeeded, with no|| 'skipped':The equality pair applies only to the publishing predecessors:
github, plustagforconda.Why it is worth fixing rather than ignoring. This comment is where the documentation got the claim from —
releasing.rstsaid the same thing, and #959 corrects it there. Once #959 merges, the docs will describe this workflow more accurately than the workflow describes itself, and the next person to reconcile the two will reasonably trust the code comment. The comment is also load-bearing in its own right: it is the record of why this spelling was chosen over!= 'failure', and that reasoning is correct — only the scope of the generalisation is wrong.Fix is one clause: name the publishing predecessors and say
version_checkis required to have strictly succeeded. No behaviour change.Note
tests/project/test_release_gates.pypins the conditions but not this: there is noversion_check.resultassertion in the file, so neither the comment nor the prose had test cover for that clause. Adding one would be a reasonable part of the same change.This touches
.github/, so it needs a pull request rather than a direct push.