-
Notifications
You must be signed in to change notification settings - Fork 23
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
Code style: Introduce Checkstyle and Spotbugs #374
Conversation
Please review @terrestris/devs |
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.
🔝 Just a few minor remarks
if (userGroup.getMembers().contains(user)) { | ||
Set<Permission> groupPermissions = groupPermissionsMap.get(userGroup).getPermissions(); | ||
Set<Permission> groupPermissions =userGroupEntry.getValue().getPermissions(); |
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.
Set<Permission> groupPermissions =userGroupEntry.getValue().getPermissions(); | |
Set<Permission> groupPermissions = userGroupEntry.getValue().getPermissions(); |
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.
👍 I've added further checkstyle rule<module name="WhitespaceAround"/>
to check for this as well.
|
||
for (Map.Entry<UserGroup, PermissionCollection> userGroupEntry: groupPermissionsMap.entrySet()) { |
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.
for (Map.Entry<UserGroup, PermissionCollection> userGroupEntry: groupPermissionsMap.entrySet()) { | |
for (Map.Entry<UserGroup, PermissionCollection> userGroupEntry : groupPermissionsMap.entrySet()) { |
} | ||
|
||
return null; | ||
return requestMappingHandlerMapping.getHandlerMethods().keySet(); |
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.
Doesn't this now blow up in case requestMappingHandlerMapping.getHandlerMethods()
is null
?
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.
No, it doesn't since getHandlerMethods
never returns null in any case.
@@ -66,7 +65,7 @@ public E cloneAndPersistTreeNode(E node) throws Exception { | |||
// recursive call for all children | |||
clonedChildren.add(cloneAndPersistTreeNode(childNode)); | |||
} | |||
((TreeFolder) node).setChildren((List<TreeNode>) children); | |||
((TreeFolder) node).setChildren((List<TreeNode>) clonedChildren); |
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.
I think @annarieger and I may recently have ran into exactly the problem you fixed here, 🔝
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.
🙏 I hope it'll fix the bug while cloning of applications
<!-- Checks for Size Violations. --> | ||
<!-- See http://checkstyle.sf.net/config_sizes.html --> | ||
<module name="FileLength"> | ||
<property name="max" value="2500"/> |
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.
I'd like to get this below 1000 at some point :-)
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.
👍 , currently "only" the HttpUtil
exceeds file length limitation of 1,000.
TXH for reviews @annarieger and @hwbllmnn |
This PR introduces static code analysis tools checkstyle and SpotBugs (the successor of FindBugs) to SHOGun-core to improve /check code style and quality.