-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Debug impls should not use Printer implementations #60
Comments
Unfortunately, the let span1 = Span::new().seconds(1).nanoseconds(1_001);
let span2 = Span::new().seconds(1).microseconds(1).nanoseconds(1);
assert_eq!(format!("{span1:?}"), format!("{span2:?}")); I mean, both spans represent the same amount of time, but their unit values are distinct. The problem is that the ISO 8601 collapses units smaller than seconds down into "fractional" seconds instead of representing them as distinct units. I think With that said, there is no specific requirement that a The reason why Jiff doesn't just show the internal struct as would be typical is because it's unwieldy and very difficult to read, especially since most units in most spans are zero. That's what I started with. While it's nicer for programmers working on Jiff, it's not nicer for users of Jiff. |
I think the question comes down to "Who will use the Debug implementations?". From my perspective, the Debug impls should be used strictly for debug purposes, i.e. if you want to know the actual content of the data for debug purposes. This is especially relevant due to what you've pointed out: the debug information currently does not provide information that would help someone debug. It may be appropriate to implement debug s.t. values set to zero are simply omitted and a |
I don't think it's quite that clear cut. While hiding units can make things harder to debug, showing too much information can also make things harder to debug. I've spent a lot of time debugging Jiff, and I can say with some confidence that showing all units (even when they are zero) makes things incredibly noisy. The output is easily one or two orders magnitude bigger than is necessary in most cases. I think the most likely path here is to use something other than ISO 8601 as the output format. But I would still want something that is information dense. |
Hm, okay. Would you consider adding a way to format it with units shown, e.g. with a new type pattern? |
Even units that are zero? I'd rather not personally. I mean I'm open to it if it becomes a widely requested feature with a solid use case. But for now, I think that can be implemented outside the crate inside the fuzzer if necessary though. |
Ah, not including units that are zero. I'll add it to the |
See #59 for an example; the printer is opinionated and produces sometimes unrepresentative output. Debug implementations should represent exactly what is stored in memory for testing purposes.
The text was updated successfully, but these errors were encountered: