Skip to content
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

contrib/utilities/update-copyright.sh: several improvements #16689

Merged
merged 1 commit into from Feb 27, 2024

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Feb 22, 2024

Separated from #16686 to facilitate easier review.

  • make lookup of last modification year more robust
  • add a --pedantic mode that greps the first year a file was modified from git log
  • restructure processing into a separate bash function and add parallel processing structure
  • run script on all files of the repository

@tamiko tamiko marked this pull request as draft February 22, 2024 08:03
@tamiko tamiko force-pushed the update_copyright_script branch 4 times, most recently from 31319d3 to 22a3452 Compare February 22, 2024 08:32
@tamiko tamiko marked this pull request as ready for review February 22, 2024 08:33
 - make lookup of last modification year more robust

 - add a --pedantic mode that greps the first year a file was modified
   from git log

 - restructure processing into a separate bash function and add parallel
   processing structure

 - run script on all files of the repository
@tamiko
Copy link
Member Author

tamiko commented Feb 22, 2024

@masterleinad I will run this script tomorrow on the header branch and verify that this indeed fixes all date ranges.

@tamiko
Copy link
Member Author

tamiko commented Feb 24, 2024

@tjhei ping

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but, wow, what a complex machinery you have built.

@tamiko tamiko merged commit aa11a9f into dealii:master Feb 27, 2024
12 of 16 checks passed
# full history:
#

[ -z "$last_year" ] && last_year=`git log --date=short --format="format:%cd %s" ${file} | \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why didn't you just write this as if test -z "$last_year" ; then ...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really purely a stylistic choice:

  • test and [ are the same shell built-in. And [ -z ... ] looks better than test -z .... I should have probably used the extended test pattern [[ ... ]] consistently...
  • I tend to use the [[ ... ]] && VARIABLE="..." a lot for simple conditional variable assignments. That's somewhat a bash coding style coming from Gentoo ebuilds. But yes, we could have used an if statement as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It's perhaps a bit harder to read, but I'm ok with the stylistic choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants