-
-
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
[hue] Reduce the number of warnings #8015
Conversation
Signed-off-by: Laurent Garnier <[email protected]>
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 some small suggestions.
@@ -42,6 +42,7 @@ public void setIpAddress(String ipAddress) { | |||
} | |||
|
|||
public int getPort() { | |||
Integer port = this.port; |
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.
Maybe better to use localPort or checkPort or something to avoid "hidding" of class member.
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.
Renamed thePort
if (scheduledFuture != null) { | ||
scheduledFuture.cancel(true); | ||
scheduledFuture = null; | ||
this.scheduledFuture = 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.
In my opinion renaming the local variable would be a better pattern instead of using this operator?
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.
Done
if (scheduledFuture != null) { | ||
scheduledFuture.cancel(true); | ||
scheduledFuture = null; | ||
this.scheduledFuture = 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.
See above
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.
Done
Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
@DerOetzi : as you reviewed and requested changes and as I handle them, can you please now approve this PR? |
LGTM |
@openhab/add-ons-maintainers : could you please review and merge this small PR ? |
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.
LGTM
Thank you |
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]> Signed-off-by: CSchlipp <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]> Signed-off-by: MPH80 <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]> Signed-off-by: Daan Meijer <[email protected]>
* Review comments considered * Last minor change Signed-off-by: Laurent Garnier <[email protected]>
Signed-off-by: Laurent Garnier [email protected]