Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Incompatible] Add Geodesics modules #112

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Incompatible] Add Geodesics modules #112

wants to merge 1 commit into from
+46 −3

Conversation

jypma
Copy link

@jypma jypma commented Jan 11, 2022

Adding them was straightforward. A few symbols resulted in duplicates
when linking, hence the extra "-D" hackery in the plugins Makefile.

falkTX
falkTX reviewed Jan 11, 2022
View reviewed changes
.gitmodules Outdated Show resolved Hide resolved
falkTX
falkTX reviewed Jan 11, 2022
View reviewed changes
doc/LICENSES.md Outdated
@@ -32,6 +32,7 @@ Bellow follows a list of all code licenses used in Cardinal and linked submodule
| Extratone | GPL-3.0-or-later | |
| Fehler Fabrik | GPL-3.0-or-later | |
| Fundamental | GPL-3.0-or-later | |
| Geodesics | GPL-3.0-or-later | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/MarcBoule/Geodesics/blob/master/plugin.json#L5 disagrees with this line, and this is a blocker

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, its LICENSE does say GPL3 as well. Why is the "or later" part a blocker?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just read up on it, thanks for clarifying. OK, well we'll at least have the PR until they do change license :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current proper term to use is GPL-3.0-only btw, this is a SPDX specification.

@falkTX
Copy link
Contributor

falkTX commented Jan 11, 2022

Geodesics uses GPLv3-only, which goes against the rules for Cardinal modules, as a way to ensure proper licenses in the long term.
See the related discussion in MarcBoule/Geodesics#4

Basically: won't merge until license situation changes. All depends on the module developers.

@jypma
Copy link
Author

jypma commented Jan 11, 2022

I've updated LICENSES.md to reflect the correct license now. Let's indeed wait to see.

@falkTX
Copy link
Contributor

falkTX commented Jan 11, 2022

A symlink is needed in plugins/res/, so we are able to use cardinal while in-tree without installing.

@falkTX falkTX force-pushed the main branch 2 times, most recently from 7f5b112 to d4ddc88 Compare January 14, 2022 02:04
@dromer
Copy link
Collaborator

dromer commented Jan 14, 2022

Sorry but I'd say close this PR until upstream has changed the license.
There are plenty of other projects we can currently focus on.

@falkTX falkTX force-pushed the main branch 3 times, most recently from 0d06beb to 4b2cb51 Compare January 14, 2022 12:20
@falkTX
Copy link
Contributor

falkTX commented Jan 14, 2022

I want to leave it open as a reminder, also the already done work is useful if the license situation changes.

@falkTX falkTX added the wontfix This will not be worked on label Jan 14, 2022
@falkTX falkTX force-pushed the main branch 3 times, most recently from e0aecb0 to 30134b4 Compare January 19, 2022 13:33
@jypma
Add Geodesics modules
ace7b1e 
Adding them was straightforward. A few symbols resulted in duplicates
when linking, hence the extra "-D" hackery in the plugins Makefile.
@falkTX falkTX changed the title Add Geodesics modules [Incompatible] Add Geodesics modules Mar 10, 2022
@falkTX falkTX force-pushed the main branch 4 times, most recently from e3644fb to ae04997 Compare August 7, 2022 15:25
@falkTX falkTX force-pushed the main branch 8 times, most recently from 0cba0a1 to 80bda51 Compare January 5, 2024 18:44
@falkTX falkTX force-pushed the main branch 4 times, most recently from 58e55b0 to 52d71a1 Compare October 6, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@falkTX falkTX falkTX left review comments

Assignees
No one assigned
Labels
wontfix This will not be worked on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jypma @falkTX @dromer