-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
ARROW-3588: [Java] Fix checkstyle for header license #2816
ARROW-3588: [Java] Fix checkstyle for header license #2816
Conversation
There was enough discrepancy in different forms of the license header, I only enable that one check here. Could those interested, check the format of the license in |
@@ -1,11 +1,13 @@ | |||
/* | |||
* Portions Copyright (C) 2017-2018 Dremio Corporation |
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.
@jacques-n this conflicted with the style check. Here I moved it down to a separate comment to get the check to pass, but let me know how you would like to format it. Alternatively, I think we could define a RegexHeader
rule that allows for an optional first line.
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.
on second thought, it's probably easiest to just suppress the check for these 2 files and keep them as they were
Just for comparison, here are the different variations of the license that I found:
|
Currently, these changes follow (1) from above, but I actually prefer (2) since it is less lines and looks a bit neater. So I don't mind changing it again. What do others think? |
I am fine with either |
#2 is more similar to the license headers used elsewhere. Either way is OK to me |
I updated the license to match (2) from above |
Hmm, I think it should not have javadoc style |
Ok, I think this is good to go |
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.
+1
Thanks @wesm and @icexelloss ! |
Enable style check for header to check the Apache license