Skip to content

Check for multiple javadocs in java headers#79603

Merged
breskeby merged 5 commits intoelastic:masterfrom
breskeby:add-check-for-duplicate-headerjavadoc
Oct 29, 2021
Merged

Check for multiple javadocs in java headers#79603
breskeby merged 5 commits intoelastic:masterfrom
breskeby:add-check-for-duplicate-headerjavadoc

Conversation

@breskeby
Copy link
Copy Markdown
Contributor

Fixes #79235

@breskeby breskeby force-pushed the add-check-for-duplicate-headerjavadoc branch from f115b08 to 0e0a853 Compare October 21, 2021 07:41
@breskeby breskeby self-assigned this Oct 21, 2021
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.16.0 v8.0.0 labels Oct 21, 2021
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just cleaned that up on the way

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We now enforce to have the license statement on the very top of the java file before the package declaration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We do some more stricter formatting on how the license statement looks. e.g. requiring a leading * and a line break after the first javadoc line /*

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This used to have two licenses declared. This seems wrong as apache license trumps sspl as far as I understand

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, unless we modified this code we should use the original license.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Another occurrence of two licenses in on java file.
If this is legit we should put them in one javadoc block. Makes it more a conscious decision two have both here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Two licenses

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed outdated elasticsearch license header

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

another two licenses

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think in this case we want to add @notice here so the original license is included in our distribution.

I'd also put our license after the orignal one and add Modifications copyright Elasticsearch B.V., then our license header.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

must be on the very top now before the package declaration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as .* is greedy here we need to make this quite explicit. Happy to have recommendations to make this simpler.

@breskeby breskeby changed the title Check for duplicate javadocs in java headers Check for multiple javadocs in java headers Oct 21, 2021
Copy link
Copy Markdown
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

👍

I noticed that the newly forked elasticsearch-lz4 library doesn't have @notice in the source license headers. I believe it should since this is vendored code. Similarly we should add the "modifications copyright elastic..." bit below the original header.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think in this case we want to add @notice here so the original license is included in our distribution.

I'd also put our license after the orignal one and add Modifications copyright Elasticsearch B.V., then our license header.

@breskeby breskeby force-pushed the add-check-for-duplicate-headerjavadoc branch from 6207f73 to 4b11697 Compare October 25, 2021 09:13
@breskeby breskeby marked this pull request as ready for review October 25, 2021 09:14
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@danhermann danhermann added v8.1.0 and removed v7.16.0 labels Oct 27, 2021
@breskeby breskeby force-pushed the add-check-for-duplicate-headerjavadoc branch from ab652ee to fd5faad Compare October 28, 2021 08:04
@breskeby breskeby merged commit 92e8ba2 into elastic:master Oct 29, 2021
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Oct 29, 2021
We also now enforce to have the license statement on the very top of the java file before 
the package declaration

Fixes elastic#79235
elasticsearchmachine pushed a commit that referenced this pull request Oct 29, 2021
We also now enforce to have the license statement on the very top of the java file before 
the package declaration

Fixes #79235
@breskeby breskeby deleted the add-check-for-duplicate-headerjavadoc branch December 10, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v8.0.0-beta1 v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wrong dual licensed source files should be detected by license header checks

5 participants