Conversation
"Second parent" is git's own term and the one the step is named after - *Find merged PullRequest
from second parent of current SHA*. "Father" is not git terminology. GHDL's integrated copy of this
logic already used `SECOND_PARENT_SHA`, so the two now agree.
Renamed in `PrepareJob.yml` (10 uses) and `PublishReleaseNotes.yml` (4 uses). Both files also had
`"{FATHER_SHA}" == ""` without the `$`, so the emptiness check compared the literal string
`{FATHER_SHA}` and never fired; that is now `"${SECOND_PARENT_SHA}"`. It was masked by the `$? -ne 0`
test in the same condition, which catches the case that actually occurs.
Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
🟢 Coverage 100.00% diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (958de9e) 45 41 91.11% Head commit (3304003) 45 (+0) 41 (+0) 91.11% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#248) 1 1 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
A status check function tolerates a *skipped* dependency - that is what it is for. It does not
distinguish between a dependency that is unrelated and one that produces the artifact the job is
about to download, so a job with `!failure() && !cancelled()` starts even when its input was never
uploaded, and fails with
Unable to download artifact(s): Artifact not found for name: documentation-HTML
Observed on GHDL's pipeline (Paebbels/ghdl run 1020): GitHub cancelled three Windows packaging jobs,
the cascade skipped the documentation job, and the publishing job ran regardless.
Three jobs in `CompletePipeline.yml` download what another job uploads, and now say so:
* `PDFDocumentation` requires `Documentation`, whose LaTeX artifact it converts.
* `PublishToGitHubPages` requires `Documentation`, `PublishCoverageResults` and `StaticTypeCheck` -
the three artifacts it assembles.
* `PublishOnPyPI` requires `Package`, whose wheel and source distribution it uploads.
`!failure() && !cancelled()` is kept alongside, so the established semantics are unchanged: a
failure anywhere in the closure still suppresses these jobs. The new terms only close the skip.
The remaining guarded jobs are left alone deliberately. `IntermediateCleanUp` and `ArtifactCleanUp`
delete artifacts and have to run whatever happened; `TriggerTaggedRelease` and `ReleasePage` create
a tag and a release page and download nothing here.
`doc/Deveopment.rst` states the distinction in the *Guidelines* section, since the rule is not
obvious from the symptom.
Co-Authored-By: Patrick Lehmann <Paebbels@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug Fixes
A job could download an artifact its producer never uploaded. A status check function tolerates a skipped dependency — that is what it is for — but it cannot distinguish a dependency that is unrelated from one that produces the artifact the job is about to download. Such a job started anyway and failed at the download:
Observed on GHDL's pipeline: GitHub cancelled three Windows packaging jobs, the cascade skipped the documentation job, and the publishing job ran regardless.
Three jobs in
CompletePipeline.ymlnow require the job producing each artifact to have succeeded:PDFDocumentationrequiresDocumentation, whose LaTeX sources it converts.PublishToGitHubPagesrequiresDocumentation,PublishCoverageResultsandStaticTypeCheck— the three HTML trees it assembles.PublishOnPyPIrequiresPackage, whose wheel and source distribution it uploads.!failure() && !cancelled()is kept alongside, so a failure anywhere in the closure still suppresses these jobs; the new terms only close the skip. None of the producers is switchable by a caller input, so this cannot disable a supported configuration.IntermediateCleanUpandArtifactCleanUpare unchanged on purpose — they delete artifacts and must run whatever happened — as areTriggerTaggedReleaseandReleasePage, which download nothing.PrepareJob.ymlandPublishReleaseNotes.ymlcompared a literal string. Both wrote"{FATHER_SHA}" == ""without the$, so that half of the emptiness check never fired. It was masked by the$? -ne 0test beside it, which catches the case that actually occurs.Changes
SECOND_PARENT_SHAinstead ofFATHER_SHA, inPrepareJob.yml(10 uses) andPublishReleaseNotes.yml(4 uses). "Second parent" is git's own term and the one the step is named after; "father" is not git terminology. GHDL's integrated copy of this logic already used that name, so the two now agree.Documentation
doc/Deveopment.rst, Conditional Jobs → Guidelines, states the distinction between an unrelated dependency and a producing one, with the observed failure as the worked example. The rule is not obvious from the symptom.Related Issues and Pull-Requests
Important
r7has to be moved to this release after merging, otherwise no consumer picks it up.