-
Notifications
You must be signed in to change notification settings - Fork 665
Add copyright year linter to npm lint command #3197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>
Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>
|
@copilot when running in GitHub CI don't we have access to the baseline reference via std env var? I would like to use that as a more reliable reference in CI than the current fallback logic. |
Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>
Updated the script in commit 0dc1f79 to use Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot the FILE_PATTERNS seems overly complicated. Would't the following suffice?
Another thing I would like to add to the linter is a check that the copyright header is not followed by a full text of Apache header, e.g. by checking that the string // SPDX-License-Identifier: Apache-2.0 and point to https://github.com/jaegertracing/jaeger-ui/blob/main/CONTRIBUTING.md#license Finally, when printing the files which are in error prefix them with the stop emoji 🛑 |
Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>
Implemented all requested changes in commit 5a69f8d:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Signed-off-by: Yuri Shkuro <github@ysh.us>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3197 +/- ##
=======================================
Coverage 97.89% 97.89%
=======================================
Files 262 262
Lines 8212 8212
Branches 2172 2141 -31
=======================================
Hits 8039 8039
Misses 172 172
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot I just added two files to this PR that should have failed in the CI. When I ran the script locally it correctly identified those files. But in GitHub actions the run did not result in an error: https://github.com/jaegertracing/jaeger-ui/actions/runs/19803541571/job/56734288664?pr=3197 |
Co-authored-by: yurishkuro <3523016+yurishkuro@users.noreply.github.com>
The issue was that |
Signed-off-by: Yuri Shkuro <github@ysh.us>
| # Check each new file for the current year in copyright header and no full Apache license | ||
| FAILED_YEAR_FILES=() | ||
| FAILED_LICENSE_FILES=() | ||
| for file in $NEW_FILES; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The $NEW_FILES variable is unquoted in the for loop, which will cause word splitting on spaces, tabs, and newlines. If any filenames contain spaces or special characters, the script will fail to check them correctly or iterate over partial filenames.
Fix by using a while loop with read:
while IFS= read -r file; do
[ -z "$file" ] && continue
if [ -f "$file" ]; then
# ... rest of the logic
fi
done <<< "$NEW_FILES"Or by quoting in the loop:
for file in "${NEW_FILES[@]}"; doThough the latter requires converting NEW_FILES to an array using readarray or mapfile.
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
check-license.shscript and copyright header formatscripts/check-copyright-year.shthat:npm lintcommand inpackage.jsonscripts/**andpackages/*/src/**Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.