Skip to content

feat(release): version advisory composite action (ci#369 consolidated) - #384

Merged
ronaldtse merged 1 commit into
mainfrom
feature/ci-369-version-advisory-v2
Aug 7, 2026
Merged

feat(release): version advisory composite action (ci#369 consolidated)#384
ronaldtse merged 1 commit into
mainfrom
feature/ci-369-version-advisory-v2

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

Closes #369. Supersedes #375.

Same feature, consolidated architecture. #375 was blocked on a stale base that would have silently removed the release_notes input (#355 floor) and used curl .../main for the script.

What lands

Piece Role
version-advisory-action/ Encapsulated composite action — consumers pin a tag, never curl from main
.github/scripts/release-version-advisory.rb Prism AST diff + multi-signal classification + bump table
8 fixtures + runner a–e original + f public-removal, g :nodoc:, h public_api.txt
rubygems-release.yml New version_advisory boolean input (default false). release_notes preserved.
test-version-advisory.yml Fixture suite CI + no-network-libs static check
docs/rubygems-release.md Inputs row + full Version advisory section

Architecture fixes vs #375

#375 This PR
curl raw.githubusercontent.com/.../main Composite action (tag-pinnable)
Would delete release_notes (stale base) Additive only — release_notes intact
String-mangled send/respond_to? visitor Prism::Visitor subclass
12-arm case/when bump table BUMP_TABLE lookup hash (OCP)
Diff.compute uses .first on reopens .last (active definition)
5 fixtures 8 fixtures covering public-removal, :nodoc:, contract file
Fingerprint comment lied about whitespace Comment matches implementation

Fixture results (local)

8 passed, 0 failed
Case Suggested
a — internal removal patch
b — public addition minor
c — public body change unknown
d — no change none
e — public signature broken (pre-1.0) minor (demoted from major)
f — public removal (pre-1.0) minor (demoted from major)
g — :nodoc: removal patch
h — unlisted (public_api.txt) removal patch

Test plan

  • ruby -c syntax OK
  • 8/8 fixtures pass byte-exact
  • CI: test-version-advisory.yml on this PR
  • Confirm release_notes ensure-release step still present in the diff
  • Post-merge: consumers opt in via with: { version_advisory: true }

Supersedes #375. Same feature, consolidated architecture:

- Encapsulated as version-advisory-action composite (no curl-from-main)
- release_notes input preserved (stale #375 base would have removed it)
- Prism::Visitor subclass (no string-mangled send/respond_to? dispatch)
- BUMP_TABLE lookup hash (OCP) replaces 12-arm case/when
- Diff.compute uses .last for reopened-class redefinitions
- body_fingerprint comment matches implementation
- 8 fixtures (was 5): +public-removal, +:nodoc:, +public_api.txt
- version_advisory input default false — no behaviour change when off

Closes #369.
@ronaldtse
ronaldtse merged commit 8676251 into main Aug 7, 2026
73 checks passed
@ronaldtse
ronaldtse deleted the feature/ci-369-version-advisory-v2 branch August 7, 2026 10:38
@opoudjis

Copy link
Copy Markdown
Contributor

Rejecting this PR as violating the standing rule you injected on ci#375 (comment) at 10:12Z — 30 minutes before this PR merged:

Standing rule: never curl raw scripts from main; encapsulate as composite actions and pin tags.

The composite action IS encapsulated and its own description literally instructs consumers to "pin a tag (e.g. @v1), never curl from main." But the reference consumer in this same PR pins @main:

# .github/workflows/rubygems-release.yml
uses: metanorma/ci/version-advisory-action@main

Per your own standing rule and #372's three-tier discipline, this should be @v1. The composite action's own documentation contradicts its reference consumer's actual pin. Please remediate.

Second instance of the same standing-rule violation in one PR-set (see the ci#383 comment for the first). Standing rules applied asymmetrically — enforced on my contributions but violated in yours — are not acceptable. Either both templates get pinned per the rule, or the rule needs revising.

Structure otherwise clean: all seven ci#375 findings substantively addressed (BUMP_TABLE lookup, Prism::Visitor subclass, .last for reopens, dead require "json" removed, three new fixtures for f-public-removal / g-nodoc-removal / h-public-api-txt).

Minor: composite reaches out of its directory tree to invoke ${{ github.action_path }}/../.github/scripts/release-version-advisory.rb. Works mechanically; a fully self-contained composite would bundle the script inside version-advisory-action/.

🤖

This was referenced Aug 11, 2026
This was referenced Aug 11, 2026
Sign up for free to 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.

Design: what a real change detector for release-version selection looks like

2 participants