Standardize license headers and fix ESLint config#2430
Standardize license headers and fix ESLint config#2430rfontanarosa wants to merge 7 commits intomasterfrom
Conversation
Summary of ChangesHello @rfontanarosa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code quality and legal clarity by standardizing license headers throughout the project. It achieves this by integrating a new ESLint plugin to automatically check and enforce the presence and format of Apache 2.0 license headers in TypeScript and JavaScript files. The changes also include necessary updates to the ESLint configuration and development dependencies to support this new enforcement, alongside minor adjustments to package management files. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully introduces eslint-plugin-headers to enforce standardized license headers and improves the NX linting configuration by allowing argument forwarding. However, there are some inconsistencies in the quote style used in the new ESLint configuration, and the license header template uses double quotes which conflicts with the existing single-quote style found in the repository (including the eslint.config.mjs file itself). Additionally, the header check is currently limited to JavaScript and TypeScript files, while the PR title suggests a broader standardization that should likely include HTML files as well.
| 'headers/header-format': [ | ||
| 'warn', | ||
| { | ||
| "source": "string", | ||
| "content": `Copyright (year) {company}. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| https://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License.`, | ||
| "variables": { | ||
| "company": "The Ground Authors" | ||
| }, | ||
| "patterns": { | ||
| "year": { | ||
| "pattern": "\\d{4}", | ||
| "defaultValue": "2026" | ||
| } | ||
| }, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
There are two issues here:
- Quote Consistency: The keys and string values in this block use double quotes, which is inconsistent with the rest of the file.
- Header Content Discrepancy: The template uses double quotes for
"License"and"AS IS", but the existing headers in the project (including the one at the top of this file) use single quotes ('License'and'AS IS'). This will cause lint warnings on all existing files.
I've updated the suggestion to use single quotes for consistency and to match the existing header style.
'headers/header-format': [
'warn',
{
source: 'string',
content: `Copyright (year) {company}.
Licensed under the Apache License, Version 2.0 (the 'License');
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
https://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an 'AS IS' BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.`,
variables: {
company: 'The Ground Authors',
},
patterns: {
year: {
pattern: '\\d{4}',
defaultValue: '2026',
},
},
},
],
eslint.config.mjs
Outdated
| import angularTemplatePlugin from '@angular-eslint/eslint-plugin-template'; | ||
| import typescriptEslintPlugin from '@typescript-eslint/eslint-plugin'; | ||
| import typescriptParser from '@typescript-eslint/parser'; | ||
| import headers from "eslint-plugin-headers"; |
There was a problem hiding this comment.
For consistency with the rest of the file and common JavaScript style (e.g., GTS), please use single quotes for imports.
| import headers from "eslint-plugin-headers"; | |
| import headers from 'eslint-plugin-headers'; |
| return newConfig; | ||
| }), | ||
| { | ||
| files: ['**/*.ts', '**/*.js'], |
There was a problem hiding this comment.
The headers plugin and rule are currently scoped only to .ts and .js files. Since the PR aims to standardize license headers and the lint command in web/package.json also targets .html files, consider applying this rule to HTML files as well. This might require moving the headers configuration to a separate block or adding it to the HTML-specific configuration block.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2430 +/- ##
=======================================
Coverage 59.12% 59.12%
=======================================
Files 110 110
Lines 2752 2752
Branches 407 407
=======================================
Hits 1627 1627
Misses 1065 1065
Partials 60 60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
closes #1833