Update release workflow to pin new sub-action refs#82
Conversation
|
/describe |
|
✅ PR description has been generated and updated! |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
Summary
This PR correctly updates the release workflow to pin the new review-pr/reply sub-action references. The changes follow the existing pattern and include proper safeguards:
✅ Correct ordering: The sed replacements are ordered from longest to shortest path to avoid partial string matches
✅ Proper validation: Each sed replacement has a corresponding grep validation to ensure the pinning succeeded
✅ Comprehensive coverage: Updates both the action definition files and the workflow that uses them
The implementation is sound and follows defensive programming practices.
Findings
No issues found. The code changes are correct and ready to merge.
Generated by cagent PR reviewer
Signed-off-by: Derek Misler <derek.misler@docker.com>
03a5787 to
3af025d
Compare
|
/review |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
Summary
The refactoring from explicit file-by-file sed commands to a pattern-based grep+sed loop is well-designed and introduces no bugs. All initial hypotheses about potential issues (grep argument order, error handling, pattern mismatches) were investigated and dismissed:
- The grep syntax is correct and standard (
grep [options] PATTERN [file...]) - Error handling works properly through the
PINNED_FILESarray length check at line 106 - Pattern matching correctly handles comments vs. actual
uses:lines - Process substitution with
set -ebehaves as intended
The new implementation is actually more robust than the old explicit sed commands, as it:
- Dynamically discovers all files needing pinning
- Provides centralized verification via the PINNED_FILES array
- Includes comprehensive grep-based validation of pinning success
- Will automatically cover new sub-actions added under
review-pr/
Findings
No issues found. The code correctly implements the intended behavior described in the PR description.
Note: This PR successfully extends the ref-pinning automation to cover review-pr/reply without requiring hardcoded file paths for future sub-actions under the review-pr/ directory.
Summary
Extends the release workflow's ref-pinning logic to cover the new
review-pr/replysub-action. Previously, onlyreview-pr/action.ymland the top-levelreview-prreference inreview-pr.ymlwere pinned on release; this PR adds equivalent pinning and verification steps forreview-pr/reply/action.ymland thedocker/cagent-action/review-pr/reply@latestreference in.github/workflows/review-pr.yml.Changes
.github/workflows/release.yml: Addssedsubstitution to pindocker/cagent-action@latestinreview-pr/reply/action.yml, addssedsubstitution to pindocker/cagent-action/review-pr/reply@latestin.github/workflows/review-pr.yml(ordered before the existingreview-pr@latestreplacement to avoid partial-match collisions), adds two newgrepverification checks for the newly pinned refs, and includesreview-pr/reply/action.ymlin the final "Pinned refs" summary output.Breaking Changes
None — this is an additive change to the release automation only.
How to Test
::error::guards firing.review-pr/reply/action.ymland.github/workflows/review-pr.ymlboth contain the versioned ref (e.g.docker/cagent-action@vX.Y.Z/docker/cagent-action/review-pr/reply@vX.Y.Z) rather than@latest.review-pr/reply/action.ymlalongside the existing files.Closes: https://github.com/docker/gordon/issues/198