-
Notifications
You must be signed in to change notification settings - Fork 501
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
Remove unused GPL-licensed code #9009
Conversation
For unknown reasons, in 2009 several files from the JDK were copied into the Dataverse codebase, instead of referenced. It appears that these classes weren't really used.
I just noticed that the ServiceLoader pattern is in use for Exporters, so it's not completely new. :) |
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.
It does indeed look like unused code (and build/test passes). FWIW, it does not look like there is any copyright/license issue - the Classpath Exception on these classes is meant to allow them to be used in other software (on a different classpath) without the GPL applying.
Thanks for the review, @qqmyers ! I wasn't aware of the Classpath Exception. |
Is there anything that I can do to get this merged? The checks passed and there was a positive review over 9 months ago… Of course I don't want to sound impatient 😉 |
Hello, |
Added to 6.1 milestone as per 2023/08/24 discussion. |
API tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9009/2/testReport/ |
@bencomp now that we've released 6.0, I'm moving waiting 6.1 issues back to the board. But could you first handle the 6.0 merge and address any EE10 issues. Thanks. |
Although the Jenkins build failed for unknown reason, I think this is ready, @scolapasta. As far as I can tell, there are no EE10 issues. |
Yes, Jenkins appears to be failing for unrelated reasons (PostgreSQL config).
|
For unknown reasons, in 2009 several files from the JDK were copied into the Dataverse codebase, instead of referenced.
It appears that these classes weren't really used.
What this PR does / why we need it:
It removes clear examples of license confusion (if not copyright infringement) caused by including GPL-licensed code in a Apache 2.0 codebase with Harvard copyright notice. I am not a lawyer, however, and wouldn't want to get involved in legal troubles.
Which issue(s) this PR closes:
Partially addresses #2576 – which had been closed "while waiting for legal advice".
Special notes for your reviewer:
If you ever want to use services and service providers, you could do it the Java way, explained at https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/ServiceLoader.html.
It appears that @landreev wanted to use the
ServiceRegistry
at some point inTabularDataFileReader.getTabDataReaderByMimeType(String mimeType)
.I'd appreciate a positive review to have this PR count towards my Hacktoberfest goal.
Suggestions on how to test this:
See that there are no compilation errors and test failures without these classes. Check that there are no more references to these classes in comments.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: