-
Notifications
You must be signed in to change notification settings - Fork 306
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
PAYARA-4034 Cleanup sonar warnings for nucleus/common/amx-core #4124
PAYARA-4034 Cleanup sonar warnings for nucleus/common/amx-core #4124
Conversation
Jenkins test please |
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.
Great work @Cousjava. Two issues that should be addressed. There was an unfinished merge and one place where change from +
to append
did flip the arguments. Added some other suggestions as well.
Maybe you should also look into your setting for automatic formatting (I assume) of class level javadoc. That was sometimes smashed together in a way I feel it should not.
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/core/AMXValidator.java
Outdated
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/core/proxy/AMXProxyHandler.java
Outdated
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/core/proxy/ProxyFactory.java
Outdated
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/impl/config/AMXConfigImpl.java
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/impl/config/AMXConfigImpl.java
Outdated
Show resolved
Hide resolved
.../common/amx-core/src/main/java/org/glassfish/admin/amx/impl/config/ConfigBeanJMXSupport.java
Outdated
Show resolved
Hide resolved
@@ -306,7 +305,7 @@ public String getAnonymousUser() { | |||
|
|||
// Get FileRealm class name | |||
String fileRealmClassName = adminFileAuthRealm.getClassname(); | |||
if (fileRealmClassName != null && !fileRealmClassName.equals(FILE_REALM_CLASSNAME)) { | |||
if (!fileRealmClassName.equals(FILE_REALM_CLASSNAME)) { |
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 reverse the arguments because we are 100% sure FILE_REALM_CLASSNAME
isn't null while getClassname()
could change and return null
at some point.
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/util/ArrayConversion.java
Outdated
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/util/jmx/JMXUtil.java
Show resolved
Hide resolved
|
||
public static final ReadWriteAttributeFilter READ_ONLY_FILTER = new ReadWriteAttributeFilter() { |
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 these all could be lambdas since it is not important that they implement the ReadWriteAttributeFilter
but the AttributeFilter
interface. If another type distinction is wanted the ReadWriteAttributeFilter
could also be made an interface I think.
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 couple typos but otherwise very awesome
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/core/AMXValidator.java
Outdated
Show resolved
Hide resolved
nucleus/common/amx-core/src/main/java/org/glassfish/admin/amx/impl/mbean/SampleImpl.java
Outdated
Show resolved
Hide resolved
a) Conflicts |
@Pandrex247 , @Cousjava compilation failure is likely to be caused by unfinished merge in one file. |
Jenkins test please |
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.
Nice.
default: | ||
theSet = new HashSet<T>(); | ||
Set<T> theSet = new HashSet<T>(); |
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.
You could use new HashSet<T>(Arrays.asList(array));
No description provided.