-
Notifications
You must be signed in to change notification settings - Fork 50
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
Small polish and performance improvements #64
base: master
Are you sure you want to change the base?
Conversation
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 for adapting to Java 8
@@ -81,4 +86,12 @@ private boolean isConfigurationReady() { | |||
return configuration != null && configuration.getSshHostname() != null && configuration.getSshUsername() != null | |||
&& configuration.getSshPrivateKey() != null && configuration.getSshPrivateKey().exists(); | |||
} | |||
|
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.
Seems like duplicated code. What about adding configuration
as argument and making the method static?
Another option is to add it as a default method to GerritConfiguration
.
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.
Thanks for found it. I hope, the better review interface and set it as static method.
Now as workaround I have migrated into imp class, and define method in interface.
Yes, it is old way, but it does not duplicate of code :)
… BTW, better realise into interface as Java 8
…eded better use the second version
Change-Id: Ic71f13a07ba7f43272a3001783e510b3a2eba3ba
Hi @jhansche , Cheers, |
Hi!
This PR includes small adapt for Java 8.
Internally, we reused this #59 PR which fixed issue #58 .
I will be happy if you upload on marketplace.atlassian.com.
Because our administrators can updated through UPM. :)
Thanks
Cheers,
Gonchik Tsymzhitov