The superseded count reported flags, not assemblies — "2" for one name - #3260
Draft
rbuergi wants to merge 1 commit into
Draft
The superseded count reported flags, not assemblies — "2" for one name#3260rbuergi wants to merge 1 commit into
rbuergi wants to merge 1 commit into
Conversation
…e name
`${#superseded[@]}` counts array ELEMENTS, and each assembly contributes two
(`--superseded-image-assembly` and its value). So a single superseded assembly
logged as:
superseding 2 image assembl(y|ies) from the reference set: MeshWeaver.Blazor.Views
Observed on MeshWeaver.Plugins#1268's green run — the build is correct, only the
narration is wrong. It is worth fixing rather than tolerating: this line is the
only evidence in the log of what the reference set dropped, so a reader
reconciling a CS0436 against it would be told two assemblies were superseded
when one was, and would go looking for the second.
Counts the names instead. Verified both ways with the same shell semantics the
lane uses: one name → 1, "A,B C" → 3. actionlint clean.
Mine, from #3225.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G4xPnGYEbdu8AVvR5jytyj
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to CI log output and the updated count aligns with the workflow’s existing argument-construction semantics.
Pull request overview
Fixes a misleading log message in the reusable module-pack workflow by counting superseded assembly names (not the --superseded-image-assembly <name> flag/value pairs) when printing the “superseding N image assemblies …” line, so the narration matches the actual arguments passed to build-project.
Changes:
- Introduce an explicit
superseded_ncounter derived from the same word-splitting semantics used to build thesupersededflag array. - Update the echo condition/message to use
superseded_ninstead of${#superseded[@]}.
File summaries
| File | Description |
|---|---|
| .github/workflows/node-repo-module-pack.yml | Corrects CI log narration by counting assembly names rather than flag/value array elements when reporting superseded image assemblies. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
|
ℹ️ Merge-queue steward: no action — removed from the queue with reason |
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.
The defect
${#superseded[@]}counts array elements, and each superseded assembly contributes two —the flag
--superseded-image-assemblyand its value. So one assembly logs as two:Observed on MeshWeaver.Plugins#1268's first green run. The build is correct — the flags are
built and passed properly, and the compile does exactly what it should. Only the narration is wrong.
Why bother
This line is the only evidence in the log of what left the reference set. Someone reconciling a
CS0436— or checking whether a superseded entry has gone stale (#3223) — reads it and is told twoassemblies were dropped when one was, then goes looking for a second that does not exist. A count
that disagrees with the list beside it is worse than no count.
The fix
Count the names, not the flags. Verified both ways under the same shell semantics the lane uses
(
${SUPERSEDED//,/ }word-splitting):actionlintclean.Provenance
Mine, from #3225 — the third defect from that change, after #3226 (a YAML insertion split
always-modules:from itsdefault:) and #3234 (SUPERSEDEDdefined on a step that never readit). Those two broke builds; this one only misreports. Recording the count here because the pattern
matters more than any of them individually: all three were introduced by the same change and each
was found by a consumer rather than by its author's own verification.
Pairs-with: none — narration only; no behaviour change.
No What's New — internal CI log text.