Skip to content

ci: comment license diffs on PRs#286

Open
willkill07 wants to merge 1 commit into
NVIDIA:mainfrom
willkill07:wkk_ci/license-diff-pr-comment
Open

ci: comment license diffs on PRs#286
willkill07 wants to merge 1 commit into
NVIDIA:mainfrom
willkill07:wkk_ci/license-diff-pr-comment

Conversation

@willkill07

@willkill07 willkill07 commented Jun 22, 2026

Copy link
Copy Markdown
Member

Overview

Update dependency license diff CI reporting so PRs get one upserted comment instead of license diff output only appearing in the runner summary.

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Details

  • Grants the reusable license diff workflow and its caller permission to write PR comments.
  • Builds a license diff comment with a stable hidden marker so each run updates the existing bot comment.
  • Wraps the lockfile license changes and status output in collapsible <details> sections.
  • Keeps license diff failures informational and reports PR comment API failures as warnings.

Where should the reviewer start?

Start with .github/workflows/ci_license_diff.yml, especially the new Upsert PR comment step. Validation used targeted workflow YAML parsing and uv run pre-commit run --files .github/workflows/ci.yaml .github/workflows/ci_license_diff.yml.

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Relates to: none

Summary by CodeRabbit

  • New Features
    • License diff comparisons are now automatically posted as comments on pull requests, making it easier to track and review license changes in your codebase.

Signed-off-by: Will Killian <wkillian@nvidia.com>
@willkill07 willkill07 requested a review from a team as a code owner June 22, 2026 14:09
@github-actions github-actions Bot added size:S PR is small Maintenance CI or Build or general repository maintenance labels Jun 22, 2026
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 13b1543d-6ce1-435b-8ac3-9fdcd6b48776

📥 Commits

Reviewing files that changed from the base of the PR and between d5c2407 and 946cc8e.

📒 Files selected for processing (2)
  • .github/workflows/ci.yaml
  • .github/workflows/ci_license_diff.yml
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
.github/workflows/*.{yml,yaml}

📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)

.github/workflows/*.{yml,yaml}: Put permissions: on each job that needs token access in GitHub Actions workflows
Avoid workflow-level permissions unless the repository intentionally centralizes them and the inheritance tradeoff is documented
Keep third-party actions pinned to full commit SHAs and preserve the readable version comment after the SHA
Prefer action-native or ecosystem-native caching over generic actions/cache
Use lockfiles or dependency manifests to drive cache invalidation in GitHub Actions workflows
Keep deploy and publish permissions isolated to the jobs that need them in GitHub Actions
Read both caller and callee when a workflow uses workflow_call in GitHub Actions
Put release-tag validation in the earliest practical caller job when the pipeline has tag-based publish behavior
Keep release-tag policy aligned with RELEASING.md: raw SemVer tags only, no leading v
contents: read is the default minimum permission for checkout-based build, test, docs, and packaging jobs
pull-requests: read is required for PR metadata lookup jobs in GitHub Actions workflows
pages: write and id-token: write should be limited to Pages deployment jobs and callers that invoke them through reusable workflows
For reusable workflows, the caller must grant every permission the called jobs require; the callee cannot elevate beyond what the caller provides
Prefer astral-sh/setup-uv cache support with cache-dependency-glob anchored to uv.lock
Prefer Swatinem/rust-cache with explicit shared-key and workspaces instead of ad hoc target-directory caching
Avoid caching generated outputs that can hide stale behavior unless the repo already relies on them deliberately

Files:

  • .github/workflows/ci.yaml
  • .github/workflows/ci_license_diff.yml
.{github/workflows/*.{yml,yaml},gitlab-ci.yml}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure CI workflows reference the same package names, install commands, and build commands as local development workflows

Files:

  • .github/workflows/ci.yaml
  • .github/workflows/ci_license_diff.yml
{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}

⚙️ CodeRabbit configuration file

{.github/**,.gitlab-ci.yml,.pre-commit-config.yaml,justfile,scripts/**}: Review automation changes for reproducibility, pinned versions where appropriate, secret handling, and consistency with the documented validation matrix.
Pay attention to commands that need generated native artifacts, FFI libraries, or platform-specific environment variables.

Files:

  • .github/workflows/ci.yaml
  • .github/workflows/ci_license_diff.yml
🧠 Learnings (1)
📚 Learning: 2026-05-03T04:23:07.497Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Flow PR: 46
File: .github/workflows/ci_rust.yml:31-64
Timestamp: 2026-05-03T04:23:07.497Z
Learning: In GitHub Actions workflow YAML, it’s valid to conditionally disable a service container by setting the service container’s `image` to an empty string (`''`) via a matrix variable (e.g., `redis_service_image: ''`). This intentionally makes the runner skip service initialization for that matrix entry rather than failing the job. When reviewing workflows, don’t flag this as an error if the workflow uses an empty `image` to disable the service on specific matrix entries (e.g., OS-specific setups); verify the `image` is sourced from the matrix variable and that the service is only expected to be available when a non-empty image is provided.

Applied to files:

  • .github/workflows/ci_license_diff.yml
🪛 zizmor (1.25.2)
.github/workflows/ci.yaml

[warning] 137-137: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

.github/workflows/ci_license_diff.yml

[warning] 33-33: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)


[info] 91-91: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🔇 Additional comments (2)
.github/workflows/ci.yaml (1)

137-137: LGTM!

.github/workflows/ci_license_diff.yml (1)

33-33: LGTM!

Also applies to: 66-66, 88-169


Walkthrough

The ci_license_diff caller job and reusable workflow gain pull-requests: write permission. The compare step receives an explicit id, exports compare_ref and diff_status outputs, and constructs a markdown comment body. A new upsert step then finds or creates a bot PR comment identified by an HTML marker and updates it via gh api.

Changes

License Diff PR Comment Feature

Layer / File(s) Summary
Permissions and step id wiring
.github/workflows/ci.yaml, .github/workflows/ci_license_diff.yml
pull-requests: write added to both the caller job and the reusable workflow; compare step gains id: compare so downstream steps can reference its outcome and outputs.
Compare step outputs and markdown construction
.github/workflows/ci_license_diff.yml
Script emits compare_ref and diff_status to $GITHUB_OUTPUT, continues on error (informational check), and assembles the markdown comment body including the lockfile diff details block and status output.
PR comment upsert via gh api
.github/workflows/ci_license_diff.yml
Derives PR number from GITHUB_REF_NAME, queries existing bot comments for the <!-- nemo-relay-license-diff --> marker, then PATCHes or POSTs the comment; all failure paths emit warnings and exit 0 to avoid blocking CI.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits format with correct type 'ci', lowercase scope omitted appropriately, concise imperative summary, no breaking change indicator needed, and is 32 characters (under 72 limit) without trailing period.
Description check ✅ Passed Description includes all required template sections: Overview with confirmation checkboxes, Details explaining changes, Where should the reviewer start guidance, and Related Issues section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@willkill07 willkill07 self-assigned this Jun 22, 2026
@willkill07 willkill07 added this to the 0.5 milestone Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintenance CI or Build or general repository maintenance size:S PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant