-
Notifications
You must be signed in to change notification settings - Fork 1.1k
DOCKER_TLS_VERIFY can be 'false' or empty. #520
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
Conversation
Signed-off-by: hongwei yi <hongweiyi@hotmail.com>
Signed-off-by: hongwei yi <hongweiyi@hotmail.com>
| this.dockerTlsVerify = BooleanUtils.toBoolean(dockerTlsVerify.trim()) | ||
| || BooleanUtils.toBoolean(dockerTlsVerify.trim(), "1", "0"); | ||
| String trimmed = dockerTlsVerify.trim(); | ||
| this.dockerTlsVerify = "true".equals(trimmed) || "1".equals(trimmed); |
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.
Looks not very good for eyes, have you looked at org.apache.commons.lang.BooleanUtils#toBooleanObject(java.lang.String) ?
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 org.apache.commons.lang.BooleanUtils#toBooleanObject(java.lang.String) is too complicated in this case, "true".equals(trimmed) || "1".equals(trimmed) is simple, fast and straight forward.
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.
Well, all Utils are available to simplify the code readability and i think one of them should work. Can't get idea what is wrong 🏧 and why utils can't be used... toBoolean() calls toBooleanObject()
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.
and imho TRUE should also work as true, so better to use existed utils, they are already verified and covers different corner cases.
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.
LOL, there always two ways. 1. make maximum use of Utils; 2. make code as simple as possible.
But I'm the latter one, I perfect has third libraries as little as possible.
As the former one, if BooleanUtils has a method to pass default value, it will looks very good for eyes, just like:
toBoolean(str, trueStr, falseStr, defaultValue);
Or if docker-java does not want user to set invalid value to DOCKER_TLS_VERIFY, it should throw exception directly.
Sorry about that. Signed-off-by: hongwei yi <hongweiyi@hotmail.com>
Current coverage is
|
|
Some problems with 3rd party libraries in nodejs : ) . http://www.haneycodes.net/npm-left-pad-have-we-forgotten-how-to-program/ |
closes #519
Signed-off-by: hongwei yi hongweiyi@hotmail.com
This change is