-
Notifications
You must be signed in to change notification settings - Fork 96
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
Enable checkstyle to detect incorrect package header #594
Conversation
d002340
to
1a40ae5
Compare
checkstyle-style.xml
Outdated
@@ -16,8 +16,6 @@ | |||
<!-- Make the @SuppressWarnings annotations available to Checkstyle --> | |||
<module name="SuppressWarningsHolder" /> | |||
|
|||
<module name="FileContentsHolder" /> |
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.
This is removed in new release. FileContentsHolder
is not required any more and just need to be removed from clients configurations.
@@ -263,6 +266,8 @@ | |||
<property name="tokens" value="VARIABLE_DEF"/> | |||
</module> | |||
|
|||
<module name="SuppressionCommentFilter"/> |
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.
SuppressionCommentFilter
is moved and now becomes a child of TreeWalker in new release
@@ -46,8 +46,6 @@ public PaginationMapper( | |||
* @param resultSet The result set to be cut down. | |||
* | |||
* @return The page of results desired. | |||
* | |||
* @throws com.yahoo.bard.webservice.web.PageNotFoundException if the page requested is past the last page of data. |
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.
Updated checkstyle detected that this method is actually not throwing exception, thus removing it:
JavadocMethod: Unused @throws tag for 'com.yahoo.bard.webservice.web.PageNotFoundException'.
@@ -29,8 +29,8 @@ | |||
*/ | |||
boolean handleRequest( | |||
RequestContext context, | |||
final DataApiRequest request, | |||
final DruidAggregationQuery<?> druidQuery, | |||
final ResponseProcessor response |
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.
@@ -366,6 +366,5 @@ public void setDeleted(int deleted) { | |||
public void setAdditionalProperty(String name, String value) { | |||
this.additionalProperties.put(name, value); | |||
} | |||
|
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.
(regexp) RegexpMultiline: Extra lines between braces
|
||
<module name="FileContentsHolder" /> | ||
<!-- Make the @SuppressWarnings annotations available to Checkstyle --> | ||
<module name="SuppressWarningsHolder" /> |
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.
Fix indentation
<!-- Make the @SuppressWarnings annotations available to Checkstyle --> | ||
<module name="SuppressWarningsHolder" /> | ||
|
||
<module name="FileContentsHolder" /> |
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.
FileContentsHolder
is removed from new release. It is not required any more and just need to be removed from clients configurations.
2fb0439
to
0ff70f9
Compare
0ff70f9
to
5fd89a5
Compare
@michael-mclawhorn and I noticed a while ago that Fili was able to pass the build with wrong package headers in some source files. This needs to be fixed, and it's fixed in this PR by adding
PackageDeclaration
checkstyle rule.In addition, checkstyle version has been bumped to the latest one(Nov, 2017), which is now able to detect more styling errors.