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

insert_license - Only first year range is replaced in existing license #64

Open
pezcode opened this issue Jan 26, 2023 · 6 comments
Open

Comments

@pezcode
Copy link

pezcode commented Jan 26, 2023

When a license exists in a file and insert_license with --use-current-year updates the year, it only does so for the first line containing a year:

 * Copyright © 2020-2022 Steve Luamo <steve@luamo.com>
 * Copyright © 2015-2022 Leve Stuamo <leve@stuamo.com>

becomes (note the 2022 in the second line):

 * Copyright © 2020-2023 Steve Luamo <steve@luamo.com>
 * Copyright © 2015-2022 Leve Stuamo <leve@stuamo.com>

This seems to be intentional since the comment in try_update_year_range() says "The change will affect only the first line containing years". But I'm not sure why this limitation exists and whether it could be lifted.

@Lucas-C
Copy link
Owner

Lucas-C commented Jan 26, 2023

Ping @aostrowski who implemented this 😊

As for myself, I'd be open to perform the changes on all matching lines in the header

@aostrowski
Copy link
Contributor

I put this in place because I assumed if multiple people are present in the header, only the one who made changes should have the year updated.

I tried to find real-world examples of files with multiple numbers in license headers. In most cases, I guess the correct behavior would be to update just the last spotted year, for instance like in those cases: https://github.com/torvalds/linux/blob/master/include/misc/altera.h, https://github.com/torvalds/linux/blob/master/include/video/sa1100fb.h.

In the first case, other companies were developing the header earlier on, so their copyrights shouldn't be extended when other developers build on their work. In the second case, an LCD controller have 4 digits in the name, which would be treated as a year instead of the actual copyright year.

I can modify the code to treat all numbers as years, or I can treat just the last one. A third option would be to just update the year on lines containing the word "Copyright". Perhaps this would be the best option.

Which approach do you think would work in most cases?

@pezcode
Copy link
Author

pezcode commented Jan 29, 2023

Thanks for the fast reply ☺️

I think finding an approach that works for everything is hard. In our case we do need support for updating multiple ranges. There are two options I can think of:

  • add another argument to select whether to update the first/last/all years (or ranges)
  • find a way to annotate years in the input license file that need updating, would also fix the LCD controller issue. Either some placeholder for "insert current year" or a way to mark "this is a year" - if neither is found revert to current behavior.

I understand if these are too intrusive for your taste, and adding the changes in our own fork wouldn't be terribly hard.

@Lucas-C
Copy link
Owner

Lucas-C commented Feb 1, 2023

Thank you both for your thoughts on the subject.

I think --use-current-year --update-all-year-ranges could be a simple solution that should please everyone.

Are you OK with that solution?
Is one of you interested in contributing a PR for this?

@pezcode
Copy link
Author

pezcode commented Feb 1, 2023

--update-all-year-ranges would work well 😌 The behavior should then match for both existing and updated licenses. Currently newly inserted licenses have all years/ranges updated, but should only update the first unless --update-all-year-ranges is given.

I can take a stab at a PR for this, might just take a few days before I can find the time.

@Lucas-C
Copy link
Owner

Lucas-C commented Feb 1, 2023

I can take a stab at a PR for this, might just take a few days before I can find the time.

Great! Sure, take your time 😊

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

No branches or pull requests

3 participants