go-licenses icon indicating copy to clipboard operation
go-licenses copied to clipboard

Configurable check

Open becoded opened this issue 1 year ago • 2 comments

Allow to configure the check action. Added flags:

  --allowed_license_names strings   list of allowed license names
  --exclude_forbidden               exclude forbidden licenses (default true)
  --exclude_notice                  exclude notice licenses
  --exclude_permissive              exclude permissive licenses
  --exclude_reciprocal              exclude reciprocal licenses
  --exclude_restricted              exclude restricted licenses
  --exclude_unencumbered            exclude unencumbered licenses

Keeping the default behaviour of just checking the forbidden licenses.

becoded avatar Sep 21 '22 14:09 becoded

Thank you for the contribution!

Let me start with high level questions.

Can you clarify the use case for allowed licenses? That seems too flexible as interface, it's probably only useful when very limited license types are allowed.

Would it be simpler asking someone with such flexible use case to directly parse the license CSV file and write custom validation?

Bobgy avatar Sep 23 '22 12:09 Bobgy

Currently, only the list of forbidden licenses, defined in google/licenseclassifier, is validated. And there is no way to change that, so it doesn't leave any room for flexibility in controlling that. So for that reason, I added the other flags (--exclude_forbidden, --exclude_notice, --exclude_permissive, --exclude_reciprocal, --exclude_restricted, --exclude_unencumbered). However, these sets are fixed. I have a case where a subset is allowed and for others, we need special approval. By allowing to create your own list of allowed licenses --allowed_license_names we are able to verify exactly what we need.

Personally, I don't think it would be simpler to parse the report. A part of this tool is written to check licenses, it makes sense to me that you can also control what is checked. Otherwise you could argue also that there is no need for the check action as you could also write a parser to check for the forbidden licenses.

becoded avatar Sep 23 '22 16:09 becoded

To explain why I'm asking, this tool was built primarily for Google's own use-cases for open source go projects, so I am seeking a balance between maintenance cost vs use-case coverage. I'm asking to learn more about why this feature is useful to you.

I think your use-case convinces me it may be useful to more people. Thank you for explaining that!

Bobgy avatar Sep 26 '22 09:09 Bobgy

Regarding the interface, do the following changes make it slightly simpler and more consistent?

  • Make exclude flags one flag that takes multiple values: --disallowed_categories, the description points to https://github.com/google/licenseclassifier/blob/e6a9bb99b5a6f71d5a34336b8245e305f5430f99/license_type.go#L180 for license categories.
  • Reduce number of words on allowed_license_names: --allowed_licenses.

Also, I think we should add validation and description that, the two flags cannot be used at the same time.

Bobgy avatar Sep 26 '22 10:09 Bobgy

I changed the interface. I did name it types instead of categories as that is what is used in multiple places in this repo and in google/licenseclassifier

becoded avatar Sep 26 '22 16:09 becoded

nit: update PR description?

Bobgy avatar Sep 27 '22 09:09 Bobgy

hey thank you for this PR and hopefully this gets merged soon, b/c we need to maintain a fork of license checker and it causes trouble with replace directives and deprecated go get / go install behaviour. basically soft-forks of "go install"-ed things are no longer a supported use-case since a few go versions so I'd really appreciate it if this goes upstream

hansinator avatar Sep 27 '22 12:09 hansinator