-
Notifications
You must be signed in to change notification settings - Fork 68
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
DEV2-3818 support insecure mode #663
Conversation
638c92d
to
5a69643
Compare
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, but I think someone who's more familiar with the plugin pitfalls should also look. |
Had some questions, nothing critical.
BTW, does this plugin have automatic tests of some sort? even just for passing params all the way?
@@ -24,6 +26,9 @@ | |||
import org.jetbrains.annotations.Nullable; | |||
|
|||
public class BinaryRun { | |||
|
|||
static final SemVer TLS_CONFIG_MIN_SUPPORTED_VERSION = SemVer.parseFromText("4.22.0"); |
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.
why? what is this? backward compatibility?
@@ -57,15 +57,14 @@ public String fetchBinary() throws NoValidBinaryToRunException { | |||
preferredBetaVersion.get().getVersion())); | |||
|
|||
if (!isBadVersion(preferredBetaVersion.get())) { | |||
return preferredBetaVersion.get().getVersionFullPath(); | |||
return preferredBetaVersion.get(); |
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.
can't find relation to this in the code, will beta still work?
@@ -74,6 +74,7 @@ class AppSettingsConfigurable : Configurable { | |||
settings.cloud2Url = settingsComponent!!.cloud2Url | |||
settings.useIJProxySettings = settingsComponent!!.useIJProxySettings | |||
settings.autoPluginUpdates = settingsComponent!!.autoPluginUpdates | |||
settings.ignoreCertificateErrors = settingsComponent!!.ignoreCertificateErrors |
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.
what does !! mean in kotlin?
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
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
No description provided.