-
Notifications
You must be signed in to change notification settings - Fork 1
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
New adoc Template; Added allowed-licenses.txt; Improved README.md… #3
New adoc Template; Added allowed-licenses.txt; Improved README.md… #3
Conversation
d-huynh
commented
Oct 9, 2020
- New adoc Template with four columns
- Added an allowed licenses list
- Improved README.md
- Update license merge and license override lists
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.
Hi @d-huynh,
thank you for your PR. I have added some comments with minor suggestions.
I think we should get rid of the code duplication in the two FreeMarker files. I don't have any FreeMarker experience, but it seems like there is a <#include> directive that could be used for this.
Also, please add an entry to the changelog.
README.md
Outdated
@@ -38,19 +38,21 @@ Add a maven profile to your `pom.xml`: | |||
<useMissingFile>true</useMissingFile> | |||
<licenseMergesUrl>classpath:license-merges.txt</licenseMergesUrl> | |||
<overrideUrl>classpath:override-licenses.txt</overrideUrl> | |||
<includedLicenses>classpath:allowed-licenses.txt</includedLicenses> |
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 code around is indented via spaces. Please also convert this indentation to spaces
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.
I saw the difference in length and changed, but didn't know they were spaces. Changed now.
EDL-1.0 | ||
EPL-1.0 | ||
EPL-2.0 | ||
GPL-2.0 |
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.
I don't think we allow GPL v2/v3 within our projects. Please sync this list with http://deviceinsight.pages.device-insight.de/project-development-guidelines/open-source-dependencies/
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.
Integrated the list into the allowed-list file
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.
Note: Janino's license is confirmed to be BSD-3-Clause here.
</#if> | ||
</#list> | ||
<#return result> | ||
</#function> |
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.
Up to this line everything is shared with the adoc-template.ftl file. Is there a possibility to create a common file for this?
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.
Done
@@ -0,0 +1,41 @@ | |||
<#-- spdxIdentifiers should contain all identifiers that are merged in license-merges.txt --> |
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.
I'm not really happy with the name of the file adoc-template-four-columns.ftl
since the other template also has four columns.
Maybe adoc-template-with-link.ftl
? Argh. Naming things is hard
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.
adoc-template-with-human-readable-lib-names.ftl
is long, but safe, don't you think? :-D
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.
Sounds good
…ght's legal guidelines * Extract template-base.ftl, which is used by both predefined templates now * Update override-licenses.txt