chore(copyright): add sync-header-years script and tests#639
chore(copyright): add sync-header-years script and tests#639SteinGabriel wants to merge 1 commit into
Conversation
|
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR establishes copyright header enforcement infrastructure across the repository and updates a public API method name. It introduces an ESLint rule and CLI tool to validate and synchronize Ping copyright headers with current years, integrated into the pre-commit workflow. Additionally, the davinci client API rename updates ChangesCopyright Header Enforcement
API Method Rename
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 unit tests (beta)
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. Comment |
30dc079 to
27deab1
Compare
|
View your CI Pipeline Execution ↗ for commit b6aa4a3
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
@forgerock/davinci-client
@forgerock/device-client
@forgerock/journey-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (23.13%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #639 +/- ##
==========================================
+ Coverage 18.07% 23.13% +5.06%
==========================================
Files 155 161 +6
Lines 24398 25602 +1204
Branches 1203 1613 +410
==========================================
+ Hits 4410 5924 +1514
+ Misses 19988 19678 -310 🚀 New features to boost your workflow:
|
|
Deployed 61e2428 to https://ForgeRock.github.io/ping-javascript-sdk/pr-639/61e242865789329a5bcdb18bb14c998893205993 branch gh-pages in ForgeRock/ping-javascript-sdk |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/copyright/sync-header-years.mjs`:
- Around line 171-178: The hasPingCopyrightHeader function currently scans every
line for MAYBE_PING_COPYRIGHT_LINE_REGEX within any comment line, which lets
matches inside the file pass check mode; change it to only inspect the file's
leading header block (the initial contiguous comment/blank lines) — iterate
lines from the top, stop scanning when you hit the first line that does not
match HEADER_COMMENT_LINE_REGEX or a blank line policy that ends the header, and
only test those header lines with MAYBE_PING_COPYRIGHT_LINE_REGEX; update
hasPingCopyrightHeader to return false if no match is found in that leading
header block.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad8dd158-9829-45ce-b608-4863322b7f53
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
eslint.config.mjslefthook.ymlpackage.jsonpackages/davinci-client/api-report/davinci-client.api.mdpackages/davinci-client/api-report/davinci-client.types.api.mdtools/copyright/sync-header-years.mjstools/copyright/sync-header-years.test.mjs
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/device-client - 0.0 KB (-10.0 KB, -100.0%) 📊 Minor Changes📉 @forgerock/device-client - 10.0 KB (-0.0 KB) ➖ No Changes➖ @forgerock/oidc-client - 35.3 KB 14 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
vatsalparikh
left a comment
There was a problem hiding this comment.
Thanks for the script!
I tested applying copyright headers with the latest fix-import-order PR and it worked smoothly, except for old files without copyright got the 2026 year.
Some comments on code below
| '**/*.test.*', | ||
| '**/*.spec.*', |
There was a problem hiding this comment.
I think we should allow both test and spec files to have updated copyright headers.
There was a problem hiding this comment.
Yeah, I think you're right. They are included now.
Thanks!
There was a problem hiding this comment.
This is almost 200 lines of code. It seems like we have separate logic for --check and --fix. We can combine those two and only apply --fix with updateCopyrightYears. We have to perform the same validation whether we call the --check or --fix, the only difference is with --fix we also need to call updateCopyrightYears.
Some code optimizations:
-
Merge hasPingCopyrightHeader (171–179) into hasInvalidPingCopyrightHeader (155–168)
Both functions iterate lines and test MAYBE_PING_COPYRIGHT_LINE_REGEX + HEADER_COMMENT_LINE_REGEX. The only difference: one returns true when the header is present, the other returns true when it's present but invalid. One function returning { present, valid } replaces both. -
Collapse three loops (22–35, 46–58, 60–69) into one
Right now run() builds stagedFileData in the first loop, then loops over it again for missing headers, then again for year updates. All three can run in a single for loop — read the file, check invalid format, check missing header, compute updated content — then branch once at the end based on checkOnly. -
Inline isFile (112–118) and safeReadUtf8 (120–126)
Each is called exactly once. isFile is 3 lines of real logic, safeReadUtf8 is 3 lines. Not worth a named function.
There was a problem hiding this comment.
Good call! I applied the changes you suggested. Merged the two header-check helpers into one inspectPingCopyrightHeader that returns { present, invalid } and collapsed the three loops into one.
Thanks for the suggestions!
27deab1 to
f749b2c
Compare
chore(copyright): wire sync script into lefthook and add package scripts feat(eslint): add inline ping-copyright rule to eslint.config.mjs fix(eslint): allow copyright header enforcement on test and spec files refactor(copyright): merge header checks and collapse staged-file loop in sync-header-years
f749b2c to
b6aa4a3
Compare
JIRA Ticket
N/A
Description
Adds automated Ping Identity copyright year management to the ping-javascript-sdk repo. A
pre-commit script rewrites stale headers in staged files and re-stages them; an ESLint rule
surfaces missing headers in-editor during lint runs.
Changes
tools/copyright/sync-header-years.mjs— new script; rewrites stale Ping-owned copyright headers instaged files (single-year → range, stale range end → current year); fails commit if a Ping
header has no valid year; runs in
--fixor--checkmodesync-header-years.test.mjs— 23 unit tests covering range updates, single-yearexpansion, idempotency, variant matching (
©,©,(c)), and exclusion ofnon-Ping/test/tooling files
eslint.config.mjslocal-rules/ping-copyrightrule (warn severity) that checks the first blockcomment of every
.ts/.tsx/.js/.jsx/.mjs/.cjssource file for aCopyright … Ping Identityheadereslint-plugin-headerincompatible with ESLint 9 flat config;eslint-plugin-headerslacks regex variable support needed for year-range patternslefthook.ymlcopyright-syncpre-commit command (node tools/copyright/sync-header-years.mjs --fix,stage_fixed: true) betweennx-syncandnx-checkpackage.jsoncopyright:syncandcopyright:checkscripts for manual runsTests
tools/copyright/sync-header-years.test.mjs— all passpatterns
How to test
1. Manual pre-commit trigger
Stage any file with a stale Ping copyright year and run
git commit. Thecopyright-synchook should rewrite and re-stage the header automatically.
2. Scripts
pnpm copyright:check # reports stale headers
pnpm copyright:sync # rewrites stale headers in place
3. ESLint
pnpm lint
Files missing a Ping Identity copyright header emit a
local-rules/ping-copyrightwarning.Summary by CodeRabbit
New Features
API Changes
Tests