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

various fixes to ament-copyright and add --allowed-licenses argument #416

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

alsora
Copy link
Contributor

@alsora alsora commented Oct 17, 2022

Signed-off-by: Alberto Soragna alberto.soragna@gmail.com

This PR addresses the bugs mentioned in #414.
This PR adds the feature described in #415

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
@superdyoll
Copy link
Contributor

superdyoll commented Jan 2, 2024

What's stopping this branch from being merged? Just re-discovered this bug myself and it'd be nice if this bug was fixed. It's hidden in the released version of ament_copyright due to the typo in add_copyright_year in main.py of the missing s in copyright_identifiers that this merge request fixes. Would prefer that ament_copyright updates the copyright year itself rather than having to write a separate script to do it.

Edit: Looks like @clalancette is the one who's managing this repo these days, so tagging them here to hopefully bump this up the agenda as I imagine I'm not the only one hitting this issue as we enter the new year.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

I think this PR does a little too much, in my opinion. To get this in, I think 3 PRs would help:

  1. Adds in --allowed-licenses, along with the changes to support that feature.
  2. A bugfix for the "copyright_identifiers" bug (this will be easy to backport)
  3. The fixes to change to a regex for the copyright information

Split up that way, I think we could get this in.

@superdyoll
Copy link
Contributor

superdyoll commented Jan 8, 2024

@clalancette have raised a new PR here #466 to fix the bug in --add-copyright-year caused by the lack of s on identifiers. Unfortunately it wasn't as simple as just typing s as the code had to be updated to work with multiple copyright notices at the same 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

Successfully merging this pull request may close these issues.

3 participants