Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

feat: use license-maven-plugin to allow generation of license file headers #103

Merged
merged 6 commits into from Jun 22, 2020

Conversation

CCFenner
Copy link
Contributor

Description

This change adds configuration for the license-maven-plugin to generate the following file header:

/*-
 * ---license-start
 * Corona-Warn-App
 * ---
 * Copyright (C) 2020 Deutsche Telekom AG and all other contributors
 * ---
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 * ---license-end
 */

Usage

Run mvn license:remove-file-header to remove old license headers and mvn license:update-file-header to generate headers everywhere (**/*.java) it is missing.

For the sake of a simple review I did not add the ~30 java files with updated header.

@jhagestedt
Copy link
Member

@kreincke think this is a nice improvement - what do you think?

@kreincke
Copy link
Member

@CCFenner / @jhagestedt
It's indeed a great advantage to automate the integration of file headers! Many thanks for this work!!

With respect to the idea, I did some research on the master branch to see what's our reality:

Using the command 'find . -type f -name "*.java" | wc -l' I counted the number of our java files = 29

Using the command 'find . -name "*.java" -type f | while read F; do echo $F; grep (C) $F; done | tee cpos.txt' I created a list of all java files and added its copyright line if it exists.

Using the command 'cat cpos.txt | grep \C) | wc -l' I counted the files with a copyright line= 28. Hence one file does not contain our file header
(see ./src/main/java/app/coronawarn/verification/controller/VerificationExceptionHandler.java )

Using the command 'cat cpos.txt | grep \C)' I reviewed the quality of the copyright lines:
1 * uses the template (instead of instantiating it) see ./src/main/java/app/coronawarn/verification/config/VerificationApplicationConfig.java
27 * use the CRL C) 2020, T-Systems International GmbH

CONCLUSION:
A) There is a need to automate the process.
B) We could erase all old file headers with disturbing manually added personal CR information.

But before we can approve the PR I need to understand how it works.

Reason: we must allow our employees/committers to add their names to the default CRL. The right to be named as author cannot be transferred (to anyone, hence also not to our company). And if the developers decided to insert their names, we must not erase such 'manually created CRLs'. Moreover, we must also allow, that they - and external committers - integrate their own deviating CRL. Example: If I wrote a file, it would contain the CRL '(C) 2020 Karsten Reincke, Deutsche Telekom AG ' (instead of TSI)

So let me ask:

  • Will the mvn plugin create the file header whenever the package is built or does it integrate it only if there still does not exist any file header?
  • If it does it once, does it decide whether there is a CRL or not, by evaluating the CRL itself or does it simply try to find/match '---license-start' (I think the latter one could work [but we take the risk, that if anyone erase the markers, his header would be overwritten], the first one not [there many other valid forms of CRLs])
  • (how) can a developer manually use/call the plugin to integrate the file header before modifying it?

Can anyone answer my questions and give me some details?

Again: Many thanks for the work. It is a win beyond this project. If we find a solution, we could offer this approach to all other maven based projects of our company [@CCFenner Hence you should add your copyright line and the license hint into your files, even if they are only 'configuration' files. The fact, that we discuss it is an indicator, that it is copyrightable ;-)])

Best regards Karsten

@CCFenner
Copy link
Contributor Author

CCFenner commented Jun 2, 2020

  • Will the mvn plugin create the file header whenever the package is built or does it integrate it only if there still does not exist any file header?

The plugin is not configured to run on any maven lifecycle phase, so no, it will only run if invoked with mvn license:update-file-header. If the command is executed, the generated header is added on every matching file (currently **/*.java) where no generated header is found. So an existing header will not be recognized. It needs to have the start/end/delimiter tags in place.

  • If it does it once, does it decide whether there is a CRL or not, by evaluating the CRL itself or does it simply try to find/match '---license-start' (I think the latter one could work [but we take the risk, that if anyone erase the markers, his header would be overwritten], the first one not [there many other valid forms of CRLs])

As stated above, the plugin needs the tags in place to insert at the correct location. Otherwise it just adds a new header.
The copyright line is fetched from the pom (organization.name && license.copyrightOwners). However, if a generated header is in place and the maven goal is invoked again, the copyright is not touched. see canUpdateCopyright

  • (how) can a developer manually use/call the plugin to integrate the file header before modifying it?

A contributor would simply call mvn license:update-file-header and add a new header on all files where it is missing. So once this PR is merged, the repository should be updated that all files contain the generated header.

@daniel-eder daniel-eder added the in review Moderators are investigating how to best proceed with the issue label Jun 3, 2020
@kreincke
Copy link
Member

@CCFenner / @jhagestedt : We just examined the proposal and we saw that it is a very good solution. In the next week we will accept the pull request, create an update-branch, and replace all existing headers by the version created by the plugin. Finally, we will adopt this technique into our general list of compliance tools for DT. Many thanks for your work!

@daniel-eder daniel-eder added in progress The issue is currently being resolved and removed in review Moderators are investigating how to best proceed with the issue labels Jun 22, 2020
Copy link
Member

@daniel-eder daniel-eder left a comment

Choose a reason for hiding this comment

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

Approved, ready to merge.

@daniel-eder daniel-eder merged commit 5c1f752 into corona-warn-app:master Jun 22, 2020
@CCFenner CCFenner deleted the licenseHeader branch June 25, 2020 09:15
@dfischer-tech dfischer-tech added this to the 1.0.1 milestone Jul 7, 2020
@dfischer-tech dfischer-tech added the enhancement New feature or request label Jul 7, 2020
@dfischer-tech dfischer-tech changed the title use license-maven-plugin to allow generation of license file headers feat: use license-maven-plugin to allow generation of license file headers Jul 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request in progress The issue is currently being resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants