-
Notifications
You must be signed in to change notification settings - Fork 648
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 licenses, make it LGPLv2.1+ contributions going forward #4219
Conversation
@MDAnalysis/coredevs I'm going to ask for a PR freeze until we merge this. Once this is merged we then need to make sure that all ongoing PRs agree to license their code under the terms of the LGPLv2.1 and the developer certificate of origin. |
Linter Bot Results:Hi @IAlibay! Thanks for making this PR. We linted your code and found the following: There are currently no issues detected! 🎉 |
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.
A few quick comments before dosing off. Thanks @IAlibay!
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #4219 +/- ##
===========================================
- Coverage 93.62% 93.62% -0.01%
===========================================
Files 193 179 -14
Lines 25294 24190 -1104
Branches 4063 4063
===========================================
- Hits 23682 22648 -1034
+ Misses 1096 1026 -70
Partials 516 516 ☔ View full report in Codecov by Sentry. |
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.
Otherwise LGTM, thanks Irfan.
@MDAnalysis/coredevs this is a big operational change, can I please have at least a couple more thumbs up before this gets merged? |
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.
Looks great @IAlibay, thanks for all your hard work. Very much appreciated.
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.
Thanks. Can you please add a CHANGELOG entry, too?
Thanks all - @orbeckst can I ask for a quick re-review and then we can go ahead and merge (that's enough approving votes). |
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.
All good from my end. Thank you!
We do need a blog post as the follow up to https://www.mdanalysis.org/2022/11/07/relicensing in order to explain what we are doing. When we have the URL we can put it into the CHANGELOG retrospectively. But I am not going to hold up the PR for a blog post.
Thanks @orbeckst - yeah I just decided to get this done so that we don't accrue any further GPLv2+ contributions. The blog post will be next but might take a bit longer. |
This license reconciles long standing issues with the way in which the MDAnalysis package was licensed.
Going forward, until such a time as the codebase is suitably relicensed, the MDAnalysis library will be packaged under the terms of the GPLv3 or any later version.
Any new contributions will be made under the conditions of the LGPLv2.1+.
PR Checklist
📚 Documentation preview 📚: https://mdanalysis--4219.org.readthedocs.build/en/4219/