cue
cue copied to clipboard
cue: float64 support for zero
Fix the error when attempting to use strconv.FormatFloat with a zero value:
cannot use 0.0 (type float) as float64 in argument 0 to strconv.FormatFloat: value was rounded down
The tests for maximum and minimum in Float64() do not allow for zero. The below check incorrectly sees zero as smaller, because the Sign() of zero (0) is less than the Sign() of smallestPosFloat64 (1). So, it was clear this logic was not accounting for zero.
if n.X.Cmp(smallestPosFloat64) == -1 {
closes #1669
Signed-off-by: Kevin Burge [email protected]
Good catch.
I will leave @mpvl to review the change itself, @kcburge, but just a comment on the DCO. The only requirement to assert the DCO is to include the trailer Signed-off-by
in the commit message (per the contribution guide). So one change that will be required here is to remove the DCO text itself from the commit message.
@mvdan strange. Not sure what's going on here, because it looks ok to me. Indeed git
can also log the trailer:
git log --pretty='%h %an %(trailers:key=Signed-off-by)'
@kcburge - please can you tweak the first line of the commit message per @mpvl's change to the title of this PR?
cue: float64 support for zero
This is inline with the contribution guide on commit messages.
Perhaps a force-push on the fixed commit message will also fix the DCO check... we'll see.
@kcburge friendly ping about Paul's comment above, when you have the time :) That way we can import this change into Gerrit and merge it.
If you don't have the time to do that in the next week or two, no worries - I can always take over this change for you if you'd prefer.
Any update on this? Honestly, as nice as the idea/vision of CUE is, with issues like this it comes across as very flimsy and unusable in practice. 😔
Figured out the DCO error; the commit author had the email [email protected]
, but the DCO had [email protected]
. The author seems to use the latter for his open source commits, and the signature is what matters more as far as I can tell. We haven't head from Kevin in a while, and we want this fix to be merged, so I've gone ahead and pushed to his fork branch to rebase the change and fix up the commit author and message.