-
Notifications
You must be signed in to change notification settings - Fork 217
fix: update-copyright script for linux compatibility #6451
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
base: master
Are you sure you want to change the base?
Conversation
eeshaanSA
left a comment
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.
@Parship12 Thanks for this. Two things:
-
Please always open an issue first. It is better to discuss the ideas, and then start working on it.
-
The script should be separately updated in one PR. This PR changes 300 files, and is difficult to review.
|
@eeshaanSA Yeah sorry for that, actually I have done 2 things in the PR:
Okay, I will create a different PR for the changes done in the script. |
|
@eeshaanSA @Parship12 quick question , doesn't the copyright year usually represent the creation year? |
I thought the same, but then I found this PR: #4745 |
New code added in 2025 and 2026 has to reflect the changes. That is why, it is recommended, to have the current year, or the range, for example 2021-2026. |
So, the motive of this PR is that the .hack/update-copyright.sh file fails? |
Yes, it was failing on Linux and also the file names were concatenate. |
|
Understood, thanks!
|
eeshaanSA
left a comment
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.
@Parship12, if we want to get this merged, as suggested, please separate the PRs. Ideally, first, only open a PR with the script changed. The script can be run later (probably by one of the maintainers), since it modifies almost the entire code base in some way.
|
@eeshaanSA Sure, I will do it by EOD. Do I need to open an issue and link it? |
38cdb0d to
08f6981
Compare
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
What this PR does:
Fixed the
update-copyright.shto work correctly:-Zfrom grep (using newline-separated output)sed -i(mac vs Linux)Why we need it: The script
grep -rlZwhich causing all filenames to concatenate into one string. Also, the script used macOSsed -i ''which is not working on linux.Before:

After:

Does this PR introduce a user-facing change?: