Skip to content

Conversation

@jamesbhobbs
Copy link
Contributor

@jamesbhobbs jamesbhobbs commented Oct 2, 2025

Fixes: OSS-110 https://linear.app/deepnote/issue/OSS-110/add-a-license

Summary by CodeRabbit

  • Chores
    • Added an automated license-check workflow that runs on pushes to main and on pull requests, with concurrency to cancel redundant runs.
    • Standardized CI setup steps before executing license checks.
    • Added a package script and dev dependency to enable CLI-based license validation.
    • No user-facing changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

📝 Walkthrough

Walkthrough

Adds a new GitHub Actions workflow .github/workflows/check-licenses.yml that runs on pushes to main and on all pull requests, with a concurrency group to cancel duplicate runs. The workflow defines a check_licenses job on ubuntu-latest that checks out the repository, runs a base setup action, and executes jlpm check-licenses. package.json gains a check-licenses script that runs license-checker-rseidelsohn; the package is added as a devDependency.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant GH as GitHub
    participant WF as Workflow: Check Licenses
    participant Runner as Runner (ubuntu-latest)
    participant Repo as Repository
    participant Setup as Base Setup Action
    participant jlpm as jlpm
    participant Tool as license-checker-rseidelsohn

    Dev->>GH: Push to main / Open PR
    GH->>WF: Trigger workflow (concurrency group enforces cancel)
    WF->>Runner: Start job check_licenses
    Runner->>Repo: actions/checkout
    Runner->>Setup: Run base setup action
    Runner->>jlpm: Execute `jlpm check-licenses`
    jlpm->>Tool: Invoke license-checker-rseidelsohn
    Tool-->>Runner: Return results (success/failure)
    Runner-->>GH: Report job status
Loading

Possibly related PRs

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The linked issue OSS-110 calls for adding the Apache 2.0 license file to the project, but the PR only adds a license-checker workflow and script without actually including the Apache 2.0 LICENSE file. Include the Apache 2.0 LICENSE file at the project root and ensure it matches OSS-110 requirements.
Out of Scope Changes Check ⚠️ Warning The PR introduces a CI workflow and package.json script changes that are unrelated to the objective of adding the Apache 2.0 license file, so these modifications fall outside the scope of OSS-110. Move or split the license-checker workflow into a separate PR and focus this one on adding the Apache 2.0 license file.
✅ 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: add license checker” accurately and concisely reflects the main change of adding a CI workflow step to check licenses without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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 61ad4cc and 36c85ab.

📒 Files selected for processing (2)
  • .github/workflows/check-licenses.yml (1 hunks)
  • package.json (3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/check-licenses.yml

[warning] 2-2: truthy value should be one of [false, true]

(truthy)


[error] 23-23: too many blank lines (1 > 0)

(empty-lines)

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

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

39-39: Fix Windows quoting in the script. The single-quoted allow-list breaks under cmd.exe, so Windows contributors can’t run the check. Switch to escaped double quotes.

-        "check-licenses": "license-checker-rseidelsohn --onlyAllow 'MIT;Apache-2.0;BSD-2-Clause;BSD-3-Clause;ISC;0BSD;CC0-1.0;Python-2.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;(MIT OR Apache-2.0);(MIT AND BSD-3-Clause);(Apache-2.0 OR MIT);(BSD-2-Clause OR MIT OR Apache-2.0);(MIT OR CC0-1.0);(MIT AND CC-BY-3.0);(Unlicense OR Apache-2.0)'",
+        "check-licenses": "license-checker-rseidelsohn --onlyAllow \"MIT;Apache-2.0;BSD-2-Clause;BSD-3-Clause;ISC;0BSD;CC0-1.0;Python-2.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;(MIT OR Apache-2.0);(MIT AND BSD-3-Clause);(Apache-2.0 OR MIT);(BSD-2-Clause OR MIT OR Apache-2.0);(MIT OR CC0-1.0);(MIT AND CC-BY-3.0);(Unlicense OR Apache-2.0)\"",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36c85ab and b2d8819.

📒 Files selected for processing (2)
  • .github/workflows/check-licenses.yml (1 hunks)
  • package.json (2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/check-licenses.yml

[warning] 2-2: truthy value should be one of [false, true]

(truthy)


[error] 24-24: too many blank lines (1 > 0)

(empty-lines)

@jamesbhobbs jamesbhobbs marked this pull request as ready for review October 2, 2025 17:42
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
package.json (1)

14-14: License mismatch with OSS-110 (Apache 2.0).

Project declares BSD-3-Clause; linked issue requires Apache-2.0. Update package metadata and add a LICENSE file.

Apply:

-    "license": "BSD-3-Clause",
+    "license": "Apache-2.0",

Also add an Apache-2.0 LICENSE file (and NOTICE if needed). I can draft it.

♻️ Duplicate comments (2)
.github/workflows/check-licenses.yml (1)

21-22: Remove extra blank line between steps.

-      - name: Base Setup
-        uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
-        
+      - name: Base Setup
+        uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1

As per static analysis hints.

package.json (1)

39-39: Fix Windows quoting and pin tool; avoid npx.

Use escaped double quotes (cmd.exe), and add license-checker-rseidelsohn as a devDependency to avoid network drift; invoke the local binary.

Script:

-        "check-licenses": "npx license-checker-rseidelsohn --onlyAllow 'MIT;Apache-2.0;BSD-2-Clause;BSD-3-Clause;ISC;0BSD;CC0-1.0;Python-2.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;(MIT OR Apache-2.0);(MIT AND BSD-3-Clause);(Apache-2.0 OR MIT);(BSD-2-Clause OR MIT OR Apache-2.0);(MIT OR CC0-1.0);(MIT AND CC-BY-3.0);(Unlicense OR Apache-2.0)'",
+        "check-licenses": "license-checker-rseidelsohn --onlyAllow \"MIT;Apache-2.0;BSD-2-Clause;BSD-3-Clause;ISC;0BSD;CC0-1.0;Python-2.0;CC-BY-3.0;CC-BY-4.0;Unlicense;BlueOak-1.0.0;(MIT OR Apache-2.0);(MIT AND BSD-3-Clause);(Apache-2.0 OR MIT);(BSD-2-Clause OR MIT OR Apache-2.0);(MIT OR CC0-1.0);(MIT AND CC-BY-3.0);(Unlicense OR Apache-2.0)\"",

Then add devDependency (pick a pinned version):

"devDependencies": {
  "...": "...",
  "license-checker-rseidelsohn": "x.y.z"
}

Verify with:

yarn dlx license-checker-rseidelsohn --version
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2d8819 and 1f36a38.

📒 Files selected for processing (2)
  • .github/workflows/check-licenses.yml (1 hunks)
  • package.json (1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/check-licenses.yml

[warning] 2-2: truthy value should be one of [false, true]

(truthy)


[error] 24-24: too many blank lines (1 > 0)

(empty-lines)

@@ -0,0 +1,24 @@
name: Check Licenses
on:
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Quote “on” to satisfy yamllint.

-on:
+'on':

As per static analysis hints.

📝 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
on:
'on':
🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 2-2: truthy value should be one of [false, true]

(truthy)

🤖 Prompt for AI Agents
.github/workflows/check-licenses.yml around line 2: the top-level YAML key on is
unquoted and flagged by yamllint; update that line to quote the key (replace on:
with "on":) so the file uses a quoted key, then run a quick yamllint check to
confirm the warning is resolved.

push:
branches: ["main"]
pull_request:
branches: ["*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Wildcard will miss branch names with slashes.

For pull_request, "*" won’t match branches like feature/foo. Use "**" or drop the filter.

-  pull_request:
-    branches: ["*"]
+  pull_request:
+    branches: ["**"]
🤖 Prompt for AI Agents
.github/workflows/check-licenses.yml around line 6: the branches filter uses "*"
which does not match branch names containing slashes (e.g., feature/foo); update
the filter to use "**" (branches: ["**"]) or remove the branches filter entirely
so pull_request triggers run for all branches with slashes, and commit the
change to the workflow file.

uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1

- name: Check Licenses
run: yarn check-licenses
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

Use jlpm for consistency with repo tooling.

Most scripts use jlpm; align here.

-        run: yarn check-licenses
+        run: jlpm check-licenses
📝 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
run: yarn check-licenses
run: jlpm check-licenses
🤖 Prompt for AI Agents
.github/workflows/check-licenses.yml around line 23: the workflow runs the
license check with "yarn check-licenses" but the repo uses jlpm for package
script execution; update the step to use "jlpm check-licenses" instead of "yarn
check-licenses" so the workflow uses the same package manager wrapper as the
rest of the project.

@andyjakubowski andyjakubowski merged commit e0156a9 into main Oct 6, 2025
6 checks passed
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