Conversation
📝 WalkthroughWalkthroughThe pull request introduces SPDX license header enforcement across the codebase through a new ESLint rule, updates the NOTICE.md generation to format through Prettier with conditional file writing, registers the rule in TypeScript and CSS ESLint configurations, and adds SPDX headers to auto-generated icon registry files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
coryrylan
commented
Apr 24, 2026
| import { existsSync, readFileSync, readdirSync, writeFileSync } from 'fs'; | ||
| import path from 'path'; | ||
| import * as url from 'url'; | ||
| import prettier from 'prettier'; |
Collaborator
Author
There was a problem hiding this comment.
Running prettier over any new notice file generation so it conforms to the repo wide rules and not create new changes after running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/core/eslint.config.js`:
- Line 13: The global ignores entry currently disables all linting for the two
files by using { ignores: ['src/icon/icons.ts', 'src/icon/server.ts'] }; instead
create a rule-specific override for local-typescript/require-spdx-header that
targets those two files so only that rule is disabled for them; update the
ESLint config to remove those file paths from the global ignores array and add
an overrides section with a glob matching the two files and set rules: {
'local-typescript/require-spdx-header': 'off' } (referencing the existing
ignores array and the rule name local-typescript/require-spdx-header to locate
where to change).
In `@projects/internals/eslint/README.md`:
- Line 77: Update the Vale vocabulary whitelist by adding the new SPDX-related
terms to the Elements vocabulary accept list: open
config/vale/styles/config/vocabularies/Elements/accept.txt and append the four
entries `SPDX`, `SPDX-FileCopyrightText`, `SPDX-License-Identifier`, and
`Apache-2.0` (one term per line) so the README's SPDX terminology is whitelisted
for Vale checks.
In `@projects/internals/eslint/src/local/require-spdx-header.js`:
- Around line 79-82: The loop that computes headerEnd (using headerStart,
headerEnd and the lines array) only advances while a line includes 'SPDX-' so it
stops when a blank line appears and can leave duplicate SPDX identifiers after
the fixer; change the logic to treat blank lines as part of the header scan by
advancing headerEnd while the current line is either empty/whitespace or
contains 'SPDX-' (i.e., skip over blanks between SPDX lines) so all existing
SPDX lines are included and removed/replaced by the fixer; apply the same change
to the other identical loop referenced around the second occurrence (the block
at lines 91-94) so both scans handle interleaved blank lines correctly.
- Around line 16-23: The regex literals for SPDX header matching duplicate the
holder/license text; update the four RegExp constructions (expectedCopyright,
expectedIdentifier, wrongStyleCopyright, wrongStyleIdentifier) to interpolate
the canonical constants (COPYRIGHT_HOLDER and LICENSE) instead of hard-coded
strings, ensuring you use the escaped forms (e.g., an existing LICENSE_ESCAPED
or escape the constant before embedding) so the single source of truth is used
for both expected and wrongStyle patterns; keep the surrounding open/wrongOpen
and close/wrongClose templates as-is and only replace the repeated literal
fragments with the constants.
In `@projects/internals/eslint/src/local/require-spdx-header.test.js`:
- Around line 75-76: Introduce a single constant (e.g., CURRENT_YEAR) at the top
of the test file and replace each repeated new Date().getFullYear() usage inside
the expected SPDX header strings with that constant (the locations shown are the
multi-line expected strings that start with "// SPDX-FileCopyrightText:
Copyright (c) ... NVIDIA CORPORATION & AFFILIATES. All rights reserved." and the
following "// SPDX-License-Identifier: Apache-2.0"); ensure CURRENT_YEAR is used
in every expected output block (the occurrences around the three other expected
blocks referenced) so the year value is centralized and consistent across tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cba6324d-5dfa-49ff-8071-2d0bb7433fa8
📒 Files selected for processing (7)
projects/core/eslint.config.jsprojects/internals/ci/notice/index.jsprojects/internals/eslint/README.mdprojects/internals/eslint/src/configs/css.jsprojects/internals/eslint/src/configs/typescript.jsprojects/internals/eslint/src/local/require-spdx-header.jsprojects/internals/eslint/src/local/require-spdx-header.test.js
coryrylan
force-pushed
the
topic-lint-fixup-1
branch
from
9152c51 to
8f52e6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
projects/internals/eslint/src/local/require-spdx-header.js (1)
79-82:⚠️ Potential issue | 🟡 MinorExpand the replacement span across blank lines inside the SPDX block.
Line 80 only advances while the current line contains
SPDX-, so a malformed header likecopyright / blank line / identifierleaves the old identifier behind after--fix.🩹 Suggested fix
let headerEnd = headerStart; - while (headerEnd < lines.length && lines[headerEnd].includes('SPDX-')) { - headerEnd++; - } + let sawSpdx = false; + while (headerEnd < lines.length) { + const current = lines[headerEnd]; + if (current.includes('SPDX-')) { + sawSpdx = true; + headerEnd++; + continue; + } + if (sawSpdx && current.trim() === '') { + headerEnd++; + continue; + } + break; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/eslint/src/local/require-spdx-header.js` around lines 79 - 82, The loop that computes headerEnd only advances for lines that include 'SPDX-', so blank lines inside a malformed SPDX block are left out of the replacement; update the loop in the block that uses headerStart/headerEnd/lines so headerEnd increments while the current line is either blank (lines[headerEnd].trim() === '') or contains 'SPDX-' (i.e., stop only when a line is neither blank nor contains 'SPDX-'), ensuring the replacement span covers SPDX identifiers separated by empty lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/internals/eslint/README.md`:
- Line 77: Update the README to document that the SPDX header rule is enforced
for CSS as well: add a bullet under the CSS section (or a short note) describing
the `local-css/require-spdx-header` rule and that it enforces the same two-line
SPDX header (`SPDX-FileCopyrightText` + `SPDX-License-Identifier: Apache-2.0`)
with year-handling behavior, and/or call out that `require-spdx-header` is
applied in both the TypeScript and CSS ESLint configs so readers know it's not
TypeScript-only.
In `@projects/internals/eslint/src/local/require-spdx-header.js`:
- Around line 112-119: extractYear currently scans the whole file and can pick
up SPDX text later in the file; change extractYear to only examine the top
header candidate by iterating from the start of lines and collecting the initial
contiguous header/comment block (stop when you hit the first blank line or a
line that is not a comment/header marker), then run the YEAR_PATTERN match
against that header block only; update references in the code that call
extractYear to pass the same lines array (no other API change required) and keep
using YEAR_PATTERN for the regex.
---
Duplicate comments:
In `@projects/internals/eslint/src/local/require-spdx-header.js`:
- Around line 79-82: The loop that computes headerEnd only advances for lines
that include 'SPDX-', so blank lines inside a malformed SPDX block are left out
of the replacement; update the loop in the block that uses
headerStart/headerEnd/lines so headerEnd increments while the current line is
either blank (lines[headerEnd].trim() === '') or contains 'SPDX-' (i.e., stop
only when a line is neither blank nor contains 'SPDX-'), ensuring the
replacement span covers SPDX identifiers separated by empty lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d1580d89-4921-4a76-abb4-d392287b8d55
📒 Files selected for processing (7)
projects/core/eslint.config.jsprojects/internals/ci/notice/index.jsprojects/internals/eslint/README.mdprojects/internals/eslint/src/configs/css.jsprojects/internals/eslint/src/configs/typescript.jsprojects/internals/eslint/src/local/require-spdx-header.jsprojects/internals/eslint/src/local/require-spdx-header.test.js
Comment on lines
+112
to
+119
| function extractYear(lines) { | ||
| for (const line of lines) { | ||
| if (line.includes('SPDX-FileCopyrightText:')) { | ||
| const match = line.match(YEAR_PATTERN); | ||
| if (match) return match[1]; | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Only preserve a year from the top header block.
extractYear() scans the entire file, so a later string or comment containing SPDX-FileCopyrightText can override the inserted year even when the file has no header at the top. Restrict this to the header candidate being rewritten, or stop once you leave the initial SPDX block.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/internals/eslint/src/local/require-spdx-header.js` around lines 112
- 119, extractYear currently scans the whole file and can pick up SPDX text
later in the file; change extractYear to only examine the top header candidate
by iterating from the start of lines and collecting the initial contiguous
header/comment block (stop when you hit the first blank line or a line that is
not a comment/header marker), then run the YEAR_PATTERN match against that
header block only; update references in the code that call extractYear to pass
the same lines array (no other API change required) and keep using YEAR_PATTERN
for the regex.
Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-lint-fixup-1
branch
from
8f52e6f to
78c9bc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
projects/internals/eslint/README.md (1)
77-77:⚠️ Potential issue | 🟡 MinorDocument the CSS rule here too.
This addition makes
require-spdx-headerlook TypeScript-only, butprojects/internals/eslint/src/configs/css.jsalso enableslocal-css/require-spdx-headerforsrc/**/*.css. Add the matching bullet under the CSS section or call out that the rule is enforced in both configs.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/eslint/README.md` at line 77, The README entry for require-spdx-header is missing mention that the same rule is applied to CSS; update the README.md to either add a bullet under the CSS section or note that projects/internals/eslint/src/configs/css.js also enables local-css/require-spdx-header for files matching src/**/*.css so readers know the rule is enforced for both TypeScript and CSS files (reference the rule names require-spdx-header and local-css/require-spdx-header and the CSS config file projects/internals/eslint/src/configs/css.js).projects/internals/eslint/src/local/require-spdx-header.js (2)
79-82:⚠️ Potential issue | 🟡 MinorFixer may leave duplicate SPDX lines when header lines are split by blanks.
headerEndstops at the first non-SPDX-line, so a header likecopyright, blank line,identifierleaves the old identifier in place after fix.🐛 Proposed fix
function buildFixer(sourceCode, patterns, existingYear, headerStart) { const lines = sourceCode.lines; let headerEnd = headerStart; - while (headerEnd < lines.length && lines[headerEnd].includes('SPDX-')) { - headerEnd++; - } + let seenSpdx = false; + while (headerEnd < lines.length) { + const current = lines[headerEnd]; + if (current.includes('SPDX-')) { + seenSpdx = true; + headerEnd++; + continue; + } + if (seenSpdx && current.trim() === '') { + headerEnd++; + continue; + } + break; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/eslint/src/local/require-spdx-header.js` around lines 79 - 82, The loop that computes headerEnd in require-spdx-header.js stops at the first non-SPDX line and thus leaves trailing SPDX identifiers when the header contains blank lines; update the while condition that advances headerEnd (which currently tests lines[headerEnd].includes('SPDX-')) to treat blank lines as part of the header as well (e.g., continue advancing while the line includes 'SPDX-' OR is empty/whitespace) so the fixer removes any old identifier lines even if separated by blank lines; modify the logic around headerStart/headerEnd (and any downstream slicing) to rely on this extended span.
112-120:⚠️ Potential issue | 🟡 MinorOnly preserve a year from the top header block.
extractYear()scans the entire file, so a later string or comment containingSPDX-FileCopyrightTextcan override the inserted year even when the file has no header at the top. Restrict the search to the header candidate being rewritten:🛡️ Proposed fix
-function extractYear(lines) { - for (const line of lines) { +function extractYear(lines, headerStart) { + for (let i = headerStart; i < lines.length; i++) { + const line = lines[i]; + if (!line.includes('SPDX-')) break; if (line.includes('SPDX-FileCopyrightText:')) { const match = line.match(YEAR_PATTERN); if (match) return match[1]; } } return null; }Then update the call site:
- const existingYear = extractYear(lines); + const existingYear = extractYear(lines, headerStart);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/internals/eslint/src/local/require-spdx-header.js` around lines 112 - 120, extractYear currently scans the entire file and may pick up SPDX markers outside the top header; change extractYear to only examine the header candidate by restricting its input to the header block being rewritten (e.g., accept and scan only headerLines or stop scanning once you leave the top comment block), and update the call site that currently passes full file lines to instead pass the header candidate lines (make sure to keep the existing YEAR_PATTERN and the function name extractYear so locating is trivial).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/internals/eslint/src/local/require-spdx-header.test.js`:
- Around line 10-18: Consolidate repeated SPDX header strings by adding helper
functions (e.g., buildTsHeader(year, holder) and buildCssHeader(year, holder) or
a single buildSpdxHeader(kind, year, holder)) and replace the literals
referenced by VALID_TS, VALID_CSS and the other repeated fixtures mentioned
(lines using VALID_TS/VALID_CSS at the other test cases) to call these helpers;
ensure the helpers emit the exact comment style for TypeScript (//) vs CSS (/*
*/), include the SPDX-FileCopyrightText and SPDX-License-Identifier lines plus
the trailing blank line, and update all tests that currently inline headers (the
variables VALID_TS, VALID_CSS and the other duplicate header usages) to use the
centralized builder so the header format is maintained consistently.
---
Duplicate comments:
In `@projects/internals/eslint/README.md`:
- Line 77: The README entry for require-spdx-header is missing mention that the
same rule is applied to CSS; update the README.md to either add a bullet under
the CSS section or note that projects/internals/eslint/src/configs/css.js also
enables local-css/require-spdx-header for files matching src/**/*.css so readers
know the rule is enforced for both TypeScript and CSS files (reference the rule
names require-spdx-header and local-css/require-spdx-header and the CSS config
file projects/internals/eslint/src/configs/css.js).
In `@projects/internals/eslint/src/local/require-spdx-header.js`:
- Around line 79-82: The loop that computes headerEnd in require-spdx-header.js
stops at the first non-SPDX line and thus leaves trailing SPDX identifiers when
the header contains blank lines; update the while condition that advances
headerEnd (which currently tests lines[headerEnd].includes('SPDX-')) to treat
blank lines as part of the header as well (e.g., continue advancing while the
line includes 'SPDX-' OR is empty/whitespace) so the fixer removes any old
identifier lines even if separated by blank lines; modify the logic around
headerStart/headerEnd (and any downstream slicing) to rely on this extended
span.
- Around line 112-120: extractYear currently scans the entire file and may pick
up SPDX markers outside the top header; change extractYear to only examine the
header candidate by restricting its input to the header block being rewritten
(e.g., accept and scan only headerLines or stop scanning once you leave the top
comment block), and update the call site that currently passes full file lines
to instead pass the header candidate lines (make sure to keep the existing
YEAR_PATTERN and the function name extractYear so locating is trivial).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b7e79aa9-24f6-47d6-98cd-40f0ccb20ae2
📒 Files selected for processing (9)
projects/core/build/icons.jsprojects/core/src/icon/icons.tsprojects/core/src/icon/server.tsprojects/internals/ci/notice/index.jsprojects/internals/eslint/README.mdprojects/internals/eslint/src/configs/css.jsprojects/internals/eslint/src/configs/typescript.jsprojects/internals/eslint/src/local/require-spdx-header.jsprojects/internals/eslint/src/local/require-spdx-header.test.js
johnyanarella
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores