Add Quantity type #129
Add Quantity type #129
Conversation
|
Thanks for the PR! some small nits, but looks good to me. |
| @@ -0,0 +1,56 @@ | |||
| package io.kubernetes.client.custom; | |||
|
|
|||
| public class BaseExponent { | |||
There was a problem hiding this comment.
Happy to add JavaDoc - to clarify, this is an across the board ask rather than just for BaseExponent?
| private final int base; | ||
|
|
||
| private final int exponent; | ||
|
|
There was a problem hiding this comment.
Line spacing is weird here. preference for:
private final int base;
private final int exponent;
...
public BaseExponent(...) {
...|
|
||
| BaseExponent that = (BaseExponent) o; | ||
|
|
||
| if (base != that.base) return false; |
There was a problem hiding this comment.
nit: Preference for:
return base == that.base && exponent == that.exponent && format == that.format;There was a problem hiding this comment.
Ironically, in its own auto-generated code, IntelliJ suggests this quick fix.
|
|
||
| @Override | ||
| public int hashCode() { | ||
| int result = base; |
There was a problem hiding this comment.
This is funky, and I'm not entirely sure that it is correct. (or at least I don't understand why it is correct...
Why not:
this.toString().hashCode();There was a problem hiding this comment.
I must confess that this was entirely auto-generated by IntelliJ, however, I don't see any error in it.
Given our use case, I think we can happily use this.toString().hashCode() as the cost is not prohibitive to us and it will be more future proof in the case of adding or removing fields.
| @@ -0,0 +1,20 @@ | |||
| package io.kubernetes.client.custom; | |||
|
|
|||
| public class Pair<L, R> { | |||
There was a problem hiding this comment.
We already depend on apache-commons, rev the version to 3.0 and use the Pair implementation from there?
| this.format = format; | ||
| } | ||
|
|
||
| public BigDecimal getBigDecimal() { |
|
LGTM, JavaDoc can wait (but yes, for all classes would be great, but I realized we haven't really done a good job of that in general, so don't want to hold this PR on that...) |
|
没有说明，根本不知道单位，建议加上java doc或者demo |
| @@ -1,109 +1,109 @@ | |||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | |||
There was a problem hiding this comment.
pom.xml is reformatted. hard to locate changes.
Adds an implementation of Kubernetes Quantity handling in the Java client.
This allows users to interact with
Quantitys as their raw value without having to parse units (e.g.1024instead of1Ki,1000instead of1e3,0.001instead of1m). This is helpful for example when you submit a value like1024Miand the Kubernetes API will return1Gi. You cannot reliably manipulate the string without parsing it.This
Quantityimplementation gives you an object containing itsBigDecimalvalue and the format of the suffix (e.g.DECIMAL_SI,BINARY_SI, orDECIMAL_EXPONENT) when parsed or the desired format to be written. Using this normalized form you can then manipulate theBigDecimalrepresentation and use theQuantityimplementation to re-serialize it in the appropriate notation.A PR will also be needed to
gento make the Swagger pre-processor not turnQuantityinto aStringbut map it to this class.Quantities are complex, to say the least, in Kubernetes but thanks to some guidance from @lavalamp and a lot of time in the Go Debugger, I think this behaves pretty accurately.