-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
feat: add module for control-flow analysis #2827
Conversation
Cool! 🥇 |
Looks like a nice Christmas present :-) |
@monperrus thanks for adding this one! Still I have a question regarding the license: the files you added do not have the license header. Does it mean they are in another license? What about the packaging of this module? What does it mean for the releases of Spoon? I really think we should be consistent on that subject: we already have a not-well-known license so let's avoid more ambiguities :) |
@surli I think in their original repo they had no header, and no licenses at all. As far as I understand, that means that we can do anything we want with them. So I agree we should put as header the license we want for spoon sub modules. |
We should have at least the agreement from @marcelinorc here and to put the original repository in the header then. |
I meant legally speaking. But yes you are right it would be more elegant. |
NO, it is the opposite, we cannot use it. By default, the source code has the same copyright than books. Thus, merging this PR is an explicit violation of the copyright of the author (something like plagia). |
Ok. I will write him an email then. |
By default, the source code has the same copyright than books.
Interesting. Do you have an authoritative URL about this?
|
Not necessarily authoritative: https://choosealicense.com/no-permission/ |
https://en.wikipedia.org/wiki/License-free_software
|
Marcelino has kindly accepted to put a MIT license on his project as can be seen here so I think we can proceed with merging once we have updated the headers of these files. |
Thank you @marcelinorc ! |
2517704
to
c7b938a
Compare
I think this should be ready for review. @pvojtechovsky or @surli WDYT? |
spoon-control-flow/pom.xml
Outdated
<version>1.2.17</version> | ||
</dependency> | ||
|
||
<dependency> |
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.
junit, only for test no? And I thought @monperrus did a PR for bringing junit in the parent pom?
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.
The PR in question is not merge-able yet #2828 if I am not mistaken.
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.
But as soon as it is possible, I agree that we should move junit in parent.
Is it possible to add a Readme.md? |
Yes definitely! |
Thanks a lot @nharrand for the last mile
|
And kudos to @marcelinorc for the foundational work.
|
original code by @marcelinorc