Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 2, 2025

Fixes OSS-107

Summary by CodeRabbit

  • Chores
    • Added an automated license compliance check to CI, adding a dedicated license-check job and a CI step to run license auditing so build runs surface disallowed or unapproved dependencies.
    • Added a local npm script to audit third‑party licenses against an approved list (excluding private packages and specified exceptions) to ensure consistent local and CI license reports.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

📝 Walkthrough

Walkthrough

Adds a CI "Check licenses" step to .github/workflows/ci.yml (runs npm run check-licenses) and introduces a top-level check_licenses workflow job that checks out the repo, sets up Node.js, runs npm ci, and executes the same script. Adds a check-licenses npm script to package.json that runs npx license-checker-rseidelsohn with an explicit allowlist and excludes private packages and specified packages. No other files or runtime behavior changed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions
  participant Job as check_licenses
  participant NPM as npm
  participant LC as license-checker-rseidelsohn

  Dev->>GH: Push / open PR
  GH->>Job: trigger check_licenses job
  Job->>Job: checkout, setup-node, npm ci
  Job->>NPM: run "npm run check-licenses"
  NPM->>LC: invoke with allowlist & exclusions
  LC-->>NPM: produce report + exit code
  alt exit code 0
    Job-->>GH: job succeeds
    GH-->>Dev: success status
  else exit code != 0
    Job-->>GH: job fails with report
    GH-->>Dev: failure status
  end
Loading

Possibly related PRs

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The changes introduce CI license checks but do not add the Apache 2.0 license file required by OSS-107, so the primary objective of adding a license is unmet. Please include an Apache 2.0 LICENSE file at the repository root to satisfy OSS-107 before merging.
Out of Scope Changes Check ⚠️ Warning All modifications focus on CI license checks and a new npm script rather than adding the actual license file, making them unrelated to the linked issue. Limit this PR to adding the Apache 2.0 license file or split the CI checks into a separate PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “ci: license checker” concisely and accurately highlights the main change of adding a CI license checking step.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9da660 and af00017.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml (1 hunks)
  • package.json (1 hunks)

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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c35f414 and 769042b.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml (1 hunks)
  • package.json (1 hunks)

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

♻️ Duplicate comments (1)
package.json (1)

2058-2058: Fix Windows quoting for --onlyAllow.

Single quotes break the allowlist on Windows shells; we still need escaped double quotes so the CI is portable.

-"check-licenses": "npx license-checker-rseidelsohn --onlyAllow 'MIT;Apache-2.0;ISC;BSD-2-Clause;BSD-3-Clause;0BSD;Python-2.0;CC0-1.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;MPL-2.0' --excludePrivatePackages --excludePackages 'bootstrap-less@3.3.8'",
+"check-licenses": "npx license-checker-rseidelsohn --onlyAllow \"MIT;Apache-2.0;ISC;BSD-2-Clause;BSD-3-Clause;0BSD;Python-2.0;CC0-1.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;MPL-2.0\" --excludePrivatePackages --excludePackages \"bootstrap-less@3.3.8\"",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 769042b and d9da660.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml (1 hunks)
  • package.json (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
.github/workflows/ci.yml

[warning] 1-1: Code style issues found in the file. Run Prettier to fix.

@jamesbhobbs jamesbhobbs marked this pull request as draft October 2, 2025 18:14
- Fix trailing whitespace in .github/workflows/ci.yml
- Add BSD and 'Apache v2' to allowed licenses (alternate naming for BSD-2-Clause and Apache-2.0)
- Exclude 4 devDependencies with WTFPL license: chai-as-promised, esbuild-plugin-less, truncate-utf8-bytes, utf8-byte-length
- Exclude eslint-plugin-local-rules (local package with UNKNOWN license)

All excluded packages are devDependencies used only for testing/building and are not bundled with the extension.
@jamesbhobbs jamesbhobbs requested a review from Artmann October 2, 2025 18:38
@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 2, 2025 18:38
@jamesbhobbs jamesbhobbs merged commit 0a39609 into main Oct 7, 2025
4 checks passed
@jamesbhobbs jamesbhobbs deleted the license-check branch October 7, 2025 11:10
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.

3 participants