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.

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 AGPL v3.0 (or later) license #636

Merged
merged 92 commits into from
Nov 6, 2023
Merged

add AGPL v3.0 (or later) license #636

merged 92 commits into from
Nov 6, 2023

Conversation

anonym-HPI
Copy link
Contributor

@anonym-HPI anonym-HPI commented Jan 31, 2023

This is the proposal for the AGPL v3.0 (GNU Affero General Public License Version 3.0 https://www.gnu.org/licenses/agpl-3.0.de.html).

Everyone that approves this Pull Request will agree to the license to have all his/her attributions to the software be published under this license.

In the future, when this PR gets through I would enable forced signed commits, to make clear that people commiting agree to publish there code under this license have the right to do so.

Especially the LICENSE-README and the text should be read. I tried to make it possible to have the images included with another license, without making it possible to exploit this, e.g. using images as proprietary binary files and someone publishing the source code to use the binary image file, without publishing the binary source code included in an image (or something similiar exploitable to undermine the license).
Also I spoke with Lukas to not include a header with the license info in every file, this seems to be also not be necessary (https://www.gnu.org/licenses/gpl-howto.html.en "Why license notices?")

@anonym-HPI anonym-HPI mentioned this pull request Jan 31, 2023
LICENSE-README Outdated Show resolved Hide resolved
LICENSE-README Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
LICENSE-README Outdated Show resolved Hide resolved
@Greenscreen23
Copy link
Contributor

The license mentions the contributers as the ones listed in the readme. We did not yet add ourselves to that list, so #639 should probably be resolved before we merge this pr.

Copy link
Member

@christianzoellner christianzoellner left a comment

Choose a reason for hiding this comment

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

Thanks for the proposal for LICENSE and the related LICENSE-README.

One specific addition that I would propose for the LICENSE-README is to add an exception for the FüSim Patient Files besides the exception for the images. I have written permission from the BABZ that we are allowed to publish the patient files as .csv, but we are not authors and thus cannot allow redistribution.

@anonym-HPI
Copy link
Contributor Author

Thanks for the proposal for LICENSE and the related LICENSE-README.

One specific addition that I would propose for the LICENSE-README is to add an exception for the FüSim Patient Files besides the exception for the images. I have written permission from the BABZ that we are allowed to publish the patient files as .csv, but we are not authors and thus cannot allow redistribution.

Thanks, we should make an exception for that. Patient templates or templates in general that can be part of an export should be excluded to be forced to be under this license.
As you said as we have no author rights to them, they should be excluded.
If someone has a good way of saying this, a change request would be good.
Question is, should we wait for the bachelor thesis of @hpistudent72 to be merged, as this changes maybe smth? Maybe we can find a wording that includes such changes.

@lukasrad02
Copy link
Contributor

In the future, when this PR gets through I would enable forced signed commits, to make clear that people commiting agree to publish there code under this license have the right to do so.

Just for clarification, since the term "signed commit" can be misunderstood: If I remember correctly, you're talking about signing off the commits (Signed-off-by, git commit -s), not about cryptographically signing the commits (GPG keys, git commit -S)?

@anonym-HPI
Copy link
Contributor Author

In the future, when this PR gets through I would enable forced signed commits, to make clear that people commiting agree to publish there code under this license have the right to do so.

Just for clarification, since the term "signed commit" can be misunderstood: If I remember correctly, you're talking about signing off the commits (Signed-off-by, git commit -s), not about cryptographically signing the commits (GPG keys, git commit -S)?

Yes

@lukasrad02 lukasrad02 mentioned this pull request Feb 2, 2023
3 tasks
@christianzoellner
Copy link
Member

Question is, should we wait for the bachelor thesis of @hpistudent72 to be merged, as this changes maybe smth? Maybe we can find a wording that includes such changes.

Not necessary to wait.

As far as I know, the current patients are hand-written based on the FüSim datasets, so currently there is no need to have a licencse exception at all. However, once the code from @hpistudent72 is merged, we have a csv proper import for this and other datasets, and it will be best to just deploy the datasets in the repo.

Either way, having a suitable generic exception in the licence that covers both images and datasets is useful.

@anonym-HPI
Copy link
Contributor Author

I have created a new commit, I am not sure if the text about "datasets" needs be that way, looking at #614, I can't see any csv included, are these always then imported at runtime or is there a way to host them, to be accessible for every exercise? If only at runtime I think this exclusion wouldn't be necessary or could be rewritten to only include datasets that can also be importet at runtime.
@christianzoellner @ClFeSc

@Nils1729 Nils1729 linked an issue Feb 7, 2023 that may be closed by this pull request
Copy link
Contributor

@ClFeSc ClFeSc left a comment

Choose a reason for hiding this comment

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

Some notes on the form of the presentation, for now.

LICENSE-README Outdated Show resolved Hide resolved
LICENSE-README Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@anonym-HPI
Copy link
Contributor Author

Have removed them and also the workflow

Thank you! Could you please also add these files to the gitignore so that they won't be committed by accident?

but created a README.md file there, so the folder stays in the repo

You could use a .gitkeep file instead. This would convey the message from the current README just by its name.

changed it

@lukasrad02
Copy link
Contributor

But did I understand correctly that when making a max width it would create scroll bars inside rows?

Yes, if we would specify a max width for the column, there would be horizontal scroll bars in every table cell that is larger than this max width. (Depending on the overflow rule, it could also just be cropped or grow out of the table and cross the border, but this is definitely something we don't want either.)

I would say we should avoid that. One scroll bar for the whole page is enough.

If you prefer this behavior, just go with the CSS rule from my first comment.

If there is a way (and I think CSS allows for it) to disable these and just let these license text be like they are right now, this would probably an improvement to licenses that have it and don't touch the others. What do you think?

I'm sorry, but I do not understand what you mean. Do you want to differentiate between different license texts (since you're saying "and don't touch the others")? If yes, how would this differentiation look like?

@anonym-HPI
Copy link
Contributor Author

But did I understand correctly that when making a max width it would create scroll bars inside rows?

Yes, if we would specify a max width for the column, there would be horizontal scroll bars in every table cell that is larger than this max width. (Depending on the overflow rule, it could also just be cropped or grow out of the table and cross the border, but this is definitely something we don't want either.)

I would say we should avoid that. One scroll bar for the whole page is enough.

If you prefer this behavior, just go with the CSS rule from my first comment.

If there is a way (and I think CSS allows for it) to disable these and just let these license text be like they are right now, this would probably an improvement to licenses that have it and don't touch the others. What do you think?

I'm sorry, but I do not understand what you mean. Do you want to differentiate between different license texts (since you're saying "and don't touch the others")? If yes, how would this differentiation look like?

Your improvement is good, with " not touching the others" I mean having your improvement for licenses that line breaks plus not having "the other" have scroll bars. Best of both, your improvement and no worsening.
It may not be possible if there is no CSS way of solving this, as we can't differantiate between the licenses with proper lines and without easily.

@anonym-HPI
Copy link
Contributor Author

@lukasrad02 I used now white-sprace:pre-wrap which fixes these very long lines and have added a list with third party inspired by or copied from list. This list probably includes more links than necessary, e.g. documentation links (but not all, if I remember, at least one I kept out). This is not the prettiest way to solve this, as you now need to manually update it.
If you have an idea for a tool that finds urls in the source code and automatically puts them in a list (e.g. json or html), this would be great. Also the file name is clunky, if anyone has a better naming scheme.

Add reference to tutorial on how to center a div to inspired-by-or-copied-from-list.html
@christianzoellner
Copy link
Member

Dear team,
@hpi-sam/bp2021hg1 @hpi-sam/bp2022hg1

In the last months I got some messages by different parties interested in the digital FüSim: The BABZ already uses it for their transport scenario, the Berliner Feuerwehr wants to use it for internal trainings, and at least one company is looking into offering emergency management trainer courses based on this software.

So I propose that we publish a license soon(ish).

Are there any important debates left before we merge the license file via this PR?

Best regards,
Christian

@anonym-HPI
Copy link
Contributor Author

Dear team, @hpi-sam/bp2021hg1 @hpi-sam/bp2022hg1

In the last months I got some messages by different parties interested in the digital FüSim: The BABZ already uses it for their transport scenario, the Berliner Feuerwehr wants to use it for internal trainings, and at least one company is looking into offering emergency management trainer courses based on this software.

So I propose that we publish a license soon(ish).

Are there any important debates left before we merge the license file via this PR?

Best regards, Christian

If there are no complaints anymore I would suggest pushing it. The dependency problem I suggest gets fixed in another PR, all PRs seem to have dependency check(s) failing.

@lukasrad02
Copy link
Contributor

Are there any important debates left before we merge the license file via this PR?

From my side, there are no concerns regarding the license itself. The only discussions left were about processes and tools.

Unfortunately, I do not have time to take a closer look at these things due to the holidays. However, they are rather easy to change later on, so I'll approve this PR.

Copy link
Contributor

@lukasrad02 lukasrad02 left a comment

Choose a reason for hiding this comment

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

Approval according to my last comment

@anonym-HPI anonym-HPI requested a review from ClFeSc September 20, 2023 13:45
@anonym-HPI
Copy link
Contributor Author

@Dassderdie @ClFeSc Do you have any comments or do you approve this?

Add DCO description and make it more clear that code published in the Pull Request is/needs to be under the license of this project to be merged.
We are using DCO robo check for that and requiring signing off from now on.

Signed-off-by: Marvin <[email protected]>
make prettier happy (max line length)

Signed-off-by: Marvin <[email protected]>
maybe this time linter?

Signed-off-by: Marvin <[email protected]>
now? linter?

Signed-off-by: Marvin <[email protected]>
no spaces at line endings

Signed-off-by: Marvin <[email protected]>
Signed-off-by: Marvin <[email protected]>
@anonym-HPI
Copy link
Contributor Author

Everyone has approved, that contributed code, as far as I can see over here (@hpistudent72 gave permission, also for his bachelor thesis code)
I have changed the template for Pull Requests a bit (see last commit) in short:
Added DCO description and make it more clear that code published in the Pull Request is/needs to be under the license of this project to be merged.
We are using DCO robo check from now on and requiring signing off from now on.
What is DCO, see here.

The DCO robo check should enforce sign offs from now on, but I also enabled it in the settings of dev and main (we have to see, if this should be disabled, maybe they conflict each other? DCO robo should allow bots without signoffs, not sure about the other setting).

Signoff check will be mandatory after this Pull Request is merged.

@anonym-HPI anonym-HPI merged commit e6be4d1 into dev Nov 6, 2023
8 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add license
9 participants