-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add a license_utils module to dcicutils (C4-1048) #265
Conversation
…epos where that exists.
Pull Request Test Coverage Report for Build 5736313253
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be smart to import/test in another repo before merging but generally looks great and super useful for sanity checking this info
LICENSE_TITLE = "The MIT License" | ||
LICENSE_FRAMEWORKS = ['python', 'javascript'] | ||
|
||
ALLOWED = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For our purposes it may be useful to be able to read this from file, so we can write top level files with this information or even a directory that can have sane defaults in utils and be overridden in other repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a matter of design, I did not want individual repos to decide to override it. We can have several named policies. But the policies need to be approved organizationally, not at the level of coding lines. I will likely add one for pipelines.
I have tried this in a number of repos (see the list on the first page). Some problems turned up that were since fixed. Others are ongoing issues for which I may create a way to mark the license as a known problem. We can talk more about this interactively perhaps sometime.
test/test_license_utils.py
Outdated
# Clean up the mess we made... | ||
for name in ['bogus_dummy', 'dummy']: | ||
LicenseFrameworkRegistry.LICENSE_FRAMEWORKS.pop(name, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use a function scope fixture for the registry no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me? Ha. I never use fixtures except when in a tool that already does, to not fight the style. I hate their scoping rules. Usually I'd reach for a context manager instead because I think their scoping rules are clearer. Or I'd use a class with methods that have natural static scope. I've rewritten this as a context manager in changes to be uploaded soon.
…ra declared licenses. Better support multiple copyright notices and licenses in a single license file.
Add contribution_utils to license_utils for more complete unit testing
…o Will's comments. Some testing additions.
…les use optional 'The' in copyright title.
…file for new version of poetry.
In service of Programmatic checking of license compatibility (C4-1048):
This adds a
license_utils
module todcicutils
that in particular will allow us to do tests like this in other repos:NOTE WELL: This relies on the
pip-licenses
being a dev dependency of any repo wanting to do that. I don't know how to export a dev dependency, and am not sure you can, so there is a little dance at the top of thelicense_utils.py
file that checks to see if the repo managed to be there at all, since we don't require it in the regular dependencies. The error message explains what needs to be done.We now have a spreadsheet tracking the utility of this because it got complex.
I have extended this to have a
contribution_utils
as well, tracking contributors so that we can review them to make sure we know who's been involved.