Skip to content

Conversation

@Ray0907
Copy link

@Ray0907 Ray0907 commented Jan 4, 2026

Summary

Extract duplicated logic from 8 dependency update workflows into a single reusable composite action.

Changes

  • Create .github/actions/update-cmake-dep/action.yml with shared update logic
  • Refactor 8 workflows to use the new action:
    • update-cares, update-libdeflate, update-zstd, update-libarchive
    • update-lolhtml, update-lshpack, update-hdrhistogram, update-highway

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026

Walkthrough

Adds a composite GitHub Action that automates updating CMake dependency COMMITs and refactors eight existing dependency-update workflows to call this action instead of running custom inline version-checking, tag/sha resolution, file edits, and PR creation steps.

Changes

Cohort / File(s) Summary
New GitHub Action
/.github/actions/update-cmake-dep/action.yml
Adds a composite action with inputs repo, cmake-file, dep-name, workflow-file and outputs current, latest, tag, updated. Performs COMMIT parsing, latest-release/tag/sha resolution (lightweight and annotated tags), in-place file update when needed, and PR creation via peter-evans/create-pull-request. Includes error handling for missing/malformed COMMIT, API failures, missing tag data, and invalid SHAs.
Dependency update workflows
/.github/workflows/update-cares.yml, /.github/workflows/update-hdrhistogram.yml, /.github/workflows/update-highway.yml, /.github/workflows/update-libarchive.yml, /.github/workflows/update-libdeflate.yml, /.github/workflows/update-lolhtml.yml, /.github/workflows/update-lshpack.yml, /.github/workflows/update-zstd.yml
Replaces each workflow's bespoke multi-step shell/JS logic (version extraction, GitHub API calls, tag/sha handling, sed edits, conditional PR creation and verbose PR bodies) with a single step invoking ./.github/actions/update-cmake-dep and passing repository-specific inputs. Review areas: ensure action inputs match expected file formats and that PR metadata/branch/path constraints remain acceptable.

Pre-merge checks

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description provides a clear summary and lists all 8 refactored workflows, but doesn't follow the repository's template structure with 'What does this PR do?' and 'How did you verify your code works?' sections. Reformat the description to match the repository template with explicit 'What does this PR do?' and 'How did you verify your code works?' sections, including verification details.
Linked Issues check ❓ Inconclusive No linked issues or related GitHub issue references are mentioned in the PR description or objectives. Consider linking related GitHub issues or pull requests that motivated this refactoring (e.g., issues about code duplication or workflow maintenance).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: consolidating duplicate update workflow logic into a reusable composite action.
Out of Scope Changes check ✅ Passed All changes appear focused on consolidating duplicate workflow logic into a reusable action; no unrelated refactoring or scope creep is evident.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 27ff6aa and c9d6629.

📒 Files selected for processing (9)
  • .github/actions/update-cmake-dep/action.yml
  • .github/workflows/update-cares.yml
  • .github/workflows/update-hdrhistogram.yml
  • .github/workflows/update-highway.yml
  • .github/workflows/update-libarchive.yml
  • .github/workflows/update-libdeflate.yml
  • .github/workflows/update-lolhtml.yml
  • .github/workflows/update-lshpack.yml
  • .github/workflows/update-zstd.yml
🔇 Additional comments (9)
.github/workflows/update-lshpack.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action with appropriate inputs for the lshpack dependency.

.github/workflows/update-hdrhistogram.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action with appropriate inputs for the hdrhistogram dependency.

.github/workflows/update-zstd.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action. The CloneZstd.cmake naming (vs Build*.cmake in other workflows) appears intentional based on how zstd is integrated.

.github/workflows/update-lolhtml.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action with appropriate inputs for the lolhtml dependency.

.github/workflows/update-libdeflate.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action with appropriate inputs for the libdeflate dependency.

.github/workflows/update-highway.yml (1)

18-23: LGTM!

The workflow correctly delegates to the reusable composite action with appropriate inputs for the highway dependency.

.github/workflows/update-libarchive.yml (1)

18-23: LGTM!

Clean migration to the composite action. The inputs are correctly specified and align with the libarchive dependency configuration. The workflow permissions (contents: write, pull-requests: write) are appropriate for the action's PR creation requirements.

.github/actions/update-cmake-dep/action.yml (2)

1-31: LGTM!

Well-structured inputs and outputs. The updated output using expression comparison is a clean approach for consumers to check if an update occurred.


123-141: LGTM!

The PR creation step is well-configured. The branch naming scheme using dep-name and run_number ensures uniqueness, and the PR body provides useful context including the compare link for reviewing upstream changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c9d6629 and 48350e9.

📒 Files selected for processing (1)
  • .github/actions/update-cmake-dep/action.yml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ShlomoCode
Repo: oven-sh/bun PR: 23515
File: .github/workflows/claude.yml:60-60
Timestamp: 2025-10-12T02:25:02.232Z
Learning: When reviewing GitHub Actions workflows, using semantic version tags (like v1, v2) for official actions from reputable organizations (e.g., actions from github/, anthropics/, etc.) is common and acceptable practice. Don't flag these as security issues requiring commit SHA pinning.
📚 Learning: 2025-10-12T02:25:02.232Z
Learnt from: ShlomoCode
Repo: oven-sh/bun PR: 23515
File: .github/workflows/claude.yml:60-60
Timestamp: 2025-10-12T02:25:02.232Z
Learning: When reviewing GitHub Actions workflows, using semantic version tags (like v1, v2) for official actions from reputable organizations (e.g., actions from github/, anthropics/, etc.) is common and acceptable practice. Don't flag these as security issues requiring commit SHA pinning.

Applied to files:

  • .github/actions/update-cmake-dep/action.yml
🔇 Additional comments (5)
.github/actions/update-cmake-dep/action.yml (5)

1-30: LGTM! Well-structured inputs and outputs.

The action defines clear, required inputs with helpful descriptions, and outputs are properly wired to step results. The updated output (line 30) correctly uses a comparison expression that will evaluate to a boolean string.


35-46: LGTM! Proper authentication implementation.

The gh_api helper function correctly includes authentication headers using github.token, preventing rate limiting issues on shared runners. This addresses the concerns from previous reviews.


66-123: LGTM! Robust tag resolution with proper error handling.

The implementation correctly:

  • Authenticates API calls (preventing rate limits)
  • URL-encodes tag names (line 79) to handle special characters
  • Distinguishes between lightweight and annotated tags (lines 97-110)
  • Validates all intermediate values before use
  • Provides informative error messages

This addresses previous review feedback and handles edge cases well.


125-133: LGTM! Update logic correctly preserves indentation.

The sed command (line 132) properly captures and reuses leading whitespace, addressing previous feedback. The pattern is consistent with the extraction logic (line 49) - both assume the same CMake file format.


134-152: LGTM! Well-configured PR creation.

The PR step includes:

  • Semantic version tag @v7 (acceptable per learnings for reputable actions)
  • Proper token usage
  • Clear commit messages with tag and SHA
  • Informative PR body with comparison and workflow links
  • Branch cleanup configuration

Comment on lines +48 to +64
# Extract the commit hash from the line after COMMIT
CURRENT_VERSION=$(awk '/[[:space:]]*COMMIT[[:space:]]*$/{getline; gsub(/^[[:space:]]+|[[:space:]]+$/,"",$0); print}' "${{ inputs.cmake-file }}")
if [ -z "$CURRENT_VERSION" ]; then
echo "Error: Could not find COMMIT line in ${{ inputs.cmake-file }}"
exit 1
fi
# Validate that it looks like a git hash
if ! [[ $CURRENT_VERSION =~ ^[0-9a-f]{40}$ ]]; then
echo "Error: Invalid git hash format in ${{ inputs.cmake-file }}"
echo "Found: $CURRENT_VERSION"
echo "Expected: 40 character hexadecimal string"
exit 1
fi
echo "current=$CURRENT_VERSION" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider documenting the expected CMake file format.

The awk pattern assumes a specific structure: the COMMIT keyword alone on one line, followed by the hash on the next line. While this works for the current use case, the tight coupling to this format could be brittle if CMake files vary.

Consider adding a comment documenting the expected format, or handle potential variations (e.g., inline COMMIT <hash>).

📝 Example documentation addition
+        # Extract commit hash from CMake file
+        # Expected format:
+        #   COMMIT
+        #   <40-char-hex-hash>
         CURRENT_VERSION=$(awk '/[[:space:]]*COMMIT[[:space:]]*$/{getline; gsub(/^[[:space:]]+|[[:space:]]+$/,"",$0); print}' "${{ inputs.cmake-file }}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Extract the commit hash from the line after COMMIT
CURRENT_VERSION=$(awk '/[[:space:]]*COMMIT[[:space:]]*$/{getline; gsub(/^[[:space:]]+|[[:space:]]+$/,"",$0); print}' "${{ inputs.cmake-file }}")
if [ -z "$CURRENT_VERSION" ]; then
echo "Error: Could not find COMMIT line in ${{ inputs.cmake-file }}"
exit 1
fi
# Validate that it looks like a git hash
if ! [[ $CURRENT_VERSION =~ ^[0-9a-f]{40}$ ]]; then
echo "Error: Invalid git hash format in ${{ inputs.cmake-file }}"
echo "Found: $CURRENT_VERSION"
echo "Expected: 40 character hexadecimal string"
exit 1
fi
echo "current=$CURRENT_VERSION" >> $GITHUB_OUTPUT
# Extract commit hash from CMake file
# Expected format:
# COMMIT
# <40-char-hex-hash>
CURRENT_VERSION=$(awk '/[[:space:]]*COMMIT[[:space:]]*$/{getline; gsub(/^[[:space:]]+|[[:space:]]+$/,"",$0); print}' "${{ inputs.cmake-file }}")
if [ -z "$CURRENT_VERSION" ]; then
echo "Error: Could not find COMMIT line in ${{ inputs.cmake-file }}"
exit 1
fi
# Validate that it looks like a git hash
if ! [[ $CURRENT_VERSION =~ ^[0-9a-f]{40}$ ]]; then
echo "Error: Invalid git hash format in ${{ inputs.cmake-file }}"
echo "Found: $CURRENT_VERSION"
echo "Expected: 40 character hexadecimal string"
exit 1
fi
echo "current=$CURRENT_VERSION" >> $GITHUB_OUTPUT
🤖 Prompt for AI Agents
In .github/actions/update-cmake-dep/action.yml around lines 48 to 64, the awk
extraction assumes the CMake file has a line with only "COMMIT" and the hash on
the next line which is brittle; update the file to (1) add a concise comment
documenting the expected CMake snippet formats (both "COMMIT" on its own line
followed by the hash and the alternative inline form "COMMIT <hash>"), and (2)
make the extraction more robust to handle both forms (detect and extract an
inline hash after COMMIT or the following-line hash, trimming whitespace) and
keep the existing 40-hex validation afterwards.

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.

1 participant