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

[build][minor] Add missing licenses #4794

Closed
wants to merge 1 commit into from

Conversation

yew1eb
Copy link
Contributor

@yew1eb yew1eb commented Oct 10, 2017

No description provided.

@StephanEwen
Copy link
Contributor

Thanks, good addition.
Have you checked whether the shell scripts still work now, or whether they get confused by the headers?

If all works, can you remove the exclusions from the RAT checks (root pom.xml) for these files, so that the presence of headers is checked in the future?

@yew1eb
Copy link
Contributor Author

yew1eb commented Oct 18, 2017

@StephanEwen, I see, we need a script to automatically check the headers licenses.
I will think about how to implement this script...

@StephanEwen
Copy link
Contributor

We already have a script, it is the RAT plugin: https://github.com/apache/flink/blob/master/pom.xml#L957

You only need to make sure that these files are not excluded from the check...

@yew1eb yew1eb force-pushed the add_missing_licenses branch 2 times, most recently from cf89ca0 to e2c2231 Compare October 20, 2017 06:21
@yew1eb
Copy link
Contributor Author

yew1eb commented Oct 20, 2017

thx, OIC, I have updated this PR. :)

@yew1eb
Copy link
Contributor Author

yew1eb commented Oct 24, 2017

@StephanEwen ping

@greghogan
Copy link
Contributor

Adding the license to browserconfig.xml looks fine, but why change the user configurations masters, slaves, and zoo.cfg? Are these even copyrightable?

@StephanEwen
Copy link
Contributor

The masters and slaves file probably does not need a license header (although the rules of when you need one and when not are not very clear to me).

I think config files frequently have a license header, so I would take the zoo.cfg change.

@yew1eb Are you okay if we merge this without the changes to masters and slaves?

@yew1eb
Copy link
Contributor Author

yew1eb commented Nov 2, 2017

@StephanEwen, @greghogan , I'm sorry to reply late, a little busy recently.

Stephan, I agree with you.

This PR has been updated.

@greghogan
Copy link
Contributor

+1

@zentol
Copy link
Contributor

zentol commented Nov 15, 2017

merging.

zentol pushed a commit to zentol/flink that referenced this pull request Nov 15, 2017
zentol pushed a commit to zentol/flink that referenced this pull request Nov 15, 2017
zentol pushed a commit to zentol/flink that referenced this pull request Nov 20, 2017
zentol pushed a commit to zentol/flink that referenced this pull request Nov 20, 2017
asfgit pushed a commit that referenced this pull request Nov 20, 2017
@asfgit asfgit closed this in 0665428 Nov 20, 2017
@yew1eb yew1eb deleted the add_missing_licenses branch January 25, 2018 12:10
glaksh100 pushed a commit to lyft/flink that referenced this pull request Jun 6, 2018
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.

4 participants