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

Update license header and remove UTF-8 header #2001

Merged
merged 2 commits into from
May 21, 2024
Merged

Conversation

cpelley
Copy link
Contributor

@cpelley cpelley commented May 20, 2024

As IAG for science code, I can attest to the fact that the license header needn't be so elaborate.
Furthermore, marking files with a UTF-8 header was more relevant in older versions of Python, specifically before Python 3.0, to ensure correct interpretation of non-ASCII characters. I can't think of a reason why this is included in IMPROVER headers.
(noticed this while adding new files as part of #2000)

Issues

Copy link
Contributor

@bayliffe bayliffe left a comment

Choose a reason for hiding this comment

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

  • You've used "improver" lower case in the new header, which looks nice, but as it's technically an acronym should we be using upper case? Not too worried, but others might be.
  • You've not modified the bin/improver or bin/improver-tests files, which have slightly different license headers, but could still be compressed.
  • This documentation doc/source/Code-Style-Guide.rst still lists the old header as being necessary for all new files. Can you please modify this to match the new header.

@cpelley
Copy link
Contributor Author

cpelley commented May 21, 2024

...should we be using upper case? Not too worried, but others might be.

Indeed. I wont get away with this 😋

You've not modified the bin/improver or bin/improver-tests files, which have slightly different license headers...

Ah, thanks. I'll take a look.

This documentation doc/source/Code-Style-Guide.rst still lists the old header...

👍

Thanks for the feedback @bayliffe

@cpelley cpelley requested a review from bayliffe May 21, 2024 07:21
Copy link
Contributor

@PaulAbernethy PaulAbernethy left a comment

Choose a reason for hiding this comment

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

LGTM

@cpelley cpelley merged commit 689cded into master May 21, 2024
16 checks passed
@cpelley cpelley deleted the UPDATE_LICENSE branch May 21, 2024 11:20
@cpelley cpelley mentioned this pull request Jul 29, 2024
MoseleyS pushed a commit to MoseleyS/improver that referenced this pull request Aug 22, 2024
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.

3 participants