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
time: print more than just "0" for a zero duration (revert?) #14058
Comments
(Somehow Google Groups ate my message, so here I go again) +1. Personally I favor s for two reasons: 1.) As an engineer if in doubt I lean towards SI units and 2.) As a user of software written in Go I don't know about the internal resolution; I feel that s as a unit will be recognized as a duration with higher likelihood. |
My vote is Also, the adjacent values on the number line are labeled as |
I would vote |
In general, Also, the godocs are explicit that the reason Lastly, |
I suspect this should be I understand why 0 had no suffix here, but I agree that it's a bit too irregular for its own good. It reminds me of ANSI C printf %#x formatting (only) 0 without the 0x prefix, which has caused no end of formatting and parsing surprises. (Go did not adopt this bug.) |
CL https://golang.org/cl/22357 mentions this issue. |
I'm seeing a lot of things break as a result of this change. I'm not convinced it is worth the pain to our users. Even if we can technically do it, it burns some goodwill for not much benefit, since programs will still need to think about older versions that returned "0". Thoughts? |
I'm in favor of reverting. Similar to locking down |
The only breaks I've seen are things like golden files for usage messages, which are both unimportant and easy to fix. I believe that the previous behavior may have been deliberate but it was certainly a bug and should be fixed. |
Reverting SGTM. We could argue that the unit doesn't matter for zeros.
|
@robpike, "certainly a bug" is unclear. We all understand the argument for why unitless 0 was used. Whether that's correct is a judgment call. I tend to agree 0s would be preferable. The issue is we might be beyond that point, having used the (defensible) unitless 0 already in seven major Go releases. The question is "is it so wrong, and is the benefit so great, to merit breaking users' programs and burning some goodwill about compatibility?" It's hard for me to see that it is. For comparison:
The 0 vs 0s here seems more like case 1 than case 2. |
It's your decision but it's sad if a fix this simple is blocked because of golden tests needing to be updated. |
From my perspective this change is on probation. We are updating broken tests in Google's sources assuming it will go in, and we'll keep the change in the Go 1.7 beta. So far I don't think we've seen any breakage other than golden files in tests. If that's the only breakage caused, then the change can stay. But if we learn of serious production problems caused by this, we should think strongly about rolling it back. The saving grace for this change is that time.Duration.String already returns any number of different suffixes in its result, making it a good default for just printing, but typically not nearly consistent enough for use in real production code, where you want to see the same units no matter what, so that different values being printed are comparable (it's too easy to miss the difference between 10m, 10ms, 10us or think that 5.2s > 4m). So there is a chance basically only tests will break. |
Leaving this as Go 1.7 because the decision should be made one way or the other and the bug closed for the release candidate. |
Internally, it seems that all breakages are indeed in tests alone and the tests have been corrected to not depend on this behavior. I vote that we keep the behavior (and not revert). |
As someone who uses Go external to Google, I have had no issues with the change in my code which uses |
Decision: we will not revert the change unless the community encounters significant issues during the beta period. |
I'm going to close this, as nobody has complained during the beta period. |
http://play.golang.org/p/pIagpaYGak
prints
It should show either ns because that's the internal resolution or s because that's the SI unit.
The text was updated successfully, but these errors were encountered: