Skip to content

Conversation

@IvanRibakov
Copy link

This PR addresses a problem that new users might face when adding this tool to an existing and reasonably large/complex code base.
Currently unknown extensions are silently ignored so after using this command for the first time it's hard to identify from a large diff which files were ignored due to unsupported extension.
What this PR does is it adds -strict mode (meant to be used together with -check) which will collect all instances of files with unknown suffixes, dump them to console and exit with code 1.

@google-cla
Copy link

google-cla bot commented Jan 28, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@flwyd
Copy link
Contributor

flwyd commented Apr 26, 2025

Thanks for contributing to addlicense! Please follow the instructions from the google-cla bot above.

I think there's merit to the idea of highlighting files which addlicense doesn't recognize. I have a couple concerns with your initial implementation:

  • I don't love the name "strict" for this. Maybe this should be a third "mode": add, check, and print-unknown.
  • It's similar to the -v verbose flag, but I'm not sure it would be great to do this automatically if that flag is set: a project might have hundreds or thousands of files that aren't supposed to get a copyright notice, e.g. JSON, image assets, PDFs, README.md…
  • Perhaps a count per file extension would satisfy your goal of alerting users to filetypes which aren't handled by this tool.
  • The approach of using a mutex to add to a "non-terminal errors" list isn't great. Can you just log the filename when you encounter it? If not, use a channel rather than shared memory for accumulating results of parallel tasks.

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.

2 participants