[OP-19821] Update tooling for TypeScript/JavaScript copyright header normalization [RE-REVIEW] - #24834
Conversation
Turns the JS and TS tasks from reporters into writers so headers can be repaired in bulk. The matcher accepts the header shapes actually in the tree, and skips vendored bundles, caches and build output, which are generated and must not be rewritten. https://community.openproject.org/wp/OP-19821
Lints the canonical header so it cannot drift again. Both generators of linked-plugins.module.ts emit it from COPYRIGHT_short, since that file is not hand-editable, and gitlab_integration stays exempt to preserve its third-party notice. https://community.openproject.org/wp/OP-19821
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes first-party JavaScript/TypeScript copyright headers to the compact //-- copyright … //++ line-comment format derived from COPYRIGHT_short, and adds enforcement/repair tooling (Rake + ESLint) so new header variants don’t accumulate.
Changes:
- Extend
rake copyright:update_js/copyright:update_typescriptto create/normalize compact headers and skip build output / known excluded paths. - Enforce the canonical header via
eslint-plugin-headersin the main frontend and the Hocuspocus extension, deriving the expected text from the repo-rootCOPYRIGHT_short. - Update generated plugin-linking templates/generators to emit the canonical header so generated files satisfy the lint rule.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/rake/task_copyright_spec.rb | Adds RSpec coverage for the updated JS/TS copyright normalization tasks (creation, normalization, exclusions, idempotency). |
| lib/tasks/copyright.rake | Implements updated globbing/exclusions and new JS/TS header matching/creation behavior, including build-output skips. |
| lib/open_project/plugins/frontend_linking/linked-plugins.styles.sass.erb | Switches generated header to be sourced from copyright_header. |
| lib/open_project/plugins/frontend_linking/linked-plugins.module.ts.erb | Switches generated header to be sourced from copyright_header and updates import quoting. |
| lib/open_project/plugins/frontend_linking/erb_context.rb | Adds copyright_header helper derived from COPYRIGHT_short for generator templates. |
| frontend/src/app/features/projects/form-helpers/form-attribute-groups.ts | Listed as changed in the PR metadata (diff not included in the prompt excerpt). |
| frontend/package.json | Adds eslint-plugin-headers dev dependency. |
| frontend/package-lock.json | Locks eslint-plugin-headers dependency. |
| frontend/eslint.config.mjs | Adds headers/header-format rule using header text derived from COPYRIGHT_short and updates ignores. |
| frontend/ci-plugins-generator.js | Prepends the canonical copyright header to the generated linked-plugins module content. |
| extensions/op-blocknote-hocuspocus/package.json | Adds eslint-plugin-headers dev dependency for the extension. |
| extensions/op-blocknote-hocuspocus/package-lock.json | Locks eslint-plugin-headers dependency for the extension. |
| extensions/op-blocknote-hocuspocus/eslint.config.mjs | Adds header-format rule (derived from root COPYRIGHT_short) and expands file globs to include TSX. |
| AGENTS.md | Documents the canonical JS/TS header format and the repair tasks to run. |
Files not reviewed (2)
- extensions/op-blocknote-hocuspocus/package-lock.json: Generated file
- frontend/package-lock.json: Generated file
Suppressed comments (1)
lib/tasks/copyright.rake:160
missing_copyright_regexpfor:js/:tscurrently matches an empty string at the beginning of the file and has emptyshebang/additionalcaptures. This meanscreate: truewill insert the header before any shebang line, breaking executable JS scripts. Align this with the shebang-preserving behavior of the other formats.
def missing_copyright_regexp(format)
case format
when :ruby, :rb
/\A(?<shebang>#![^\n]+\n\n?)?(?<additional># frozen_string_literal: (?:true|false)\n\n?)?\n*/m
when :js, :ts
/\A(?<shebang>)(?<additional>)/
else
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def global_excluded_globs | ||
| %w[ | ||
| frontend/node_modules/**/* | ||
| **/node_modules/**/* | ||
| tmp/**/* | ||
| modules/gitlab_integration/**/* |
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
Warning
The original PR #24573 was merged inadvertently/prematurely. This PR has been re-created to allow for a proper review.
Ticket
https://community.openproject.org/wp/OP-19821
What are you trying to accomplish?
Standardize first-party JavaScript and TypeScript copyright headers on the compact line-comment format sourced from
COPYRIGHT_short.This also makes the format automatically repairable through Rake and ESLint so new variants do not continue to accumulate.
What approach did you choose and why?
Extend the existing copyright Rake tasks to create and normalize headers for JS and TS file variants, matching the header shapes actually present in the tree rather than the canonical one alone. Path scoping and existing exclusions are preserved, and generated output — vendored bundles, dependency caches,
public/assets,frontend/out-tsc— is skipped explicitly, so the tasks never rewrite a built artefact. Existing eligible sources are normalized here so strict enforcement starts from a clean baseline.Add
eslint-plugin-headersto the main frontend and Hocuspocus lint configurations. Both derive the expected text from the rootCOPYRIGHT_short, keeping that file as the sole legal-text source — as do the two generators oflinked-plugins.module.ts, so the generated file can satisfy the rule rather than failing it. The distinct attribution undermodules/gitlab_integrationis excluded from the Rake tasks and ESLint alike.Merge checklist
AGENTS.md/CLAUDE.md)