On Wed, Aug 24, 2016 at 10:36 PM, Gerdan Santos <ger...@gmail.com> wrote:

> The following review has been posted through the commitfest application:
> make installcheck-world:  tested, passed
> Implements feature:       tested, passed
> Spec compliant:           tested, passed
> Documentation:            tested, passed
> Sorry, my mistake!
> I could not find a way to disable this functionality , I see that the
> impact can be big as it is changed the output structure \timing without a
> mode to disable it. I even finding great functionality but need a way to
> set to default.
Thanks for reviewing! I'm not really sure how to proceed, so I'll try to
summarize where it stands. Apologies if I
mischaracterize/misattribute/misremember someone's position.

Generally speaking, people disliked the third mode for \timing, and were
generally fine with AndrewG's idea of printing the timing in both raw
milliseconds and a more human-digestible format, which means that we can:

1. keep the format exactly as is, ignoring locale issues
     + It's already done
     + lightweight
     +TomL believes there will be no confusion
     - others disagree
2. we fish out the proper locale-specific abbreviations for
     + no additional settings
     + locale stuff can't be that hard
     - I've never dealt with it (American, surprise)
3. ignore locales and fall back to a left-trimmed DDD HH:MM:SS.mmm format
     + Easy to revert to that code
     + My original format and one PeterE advocated
     - others disliked
4. we have a \pset that sets fixed scale (seconds, minutes, hours, days),
sliding scale (what's displayed now), or interval
     + some flexibility with some easy config values
     - still have the locale issue
     -  likely will miss a format somebody wanted
4. The \pset option is a time format string like "%d %h:%m:%s".
     + maximum flexibility
     + sidesteps the locale issue by putting it in the user's hands
     - those format strings are sometimes tough for users to grok
     - ISO 8601 isn't much of a help as it doesn't handle milliseconds
     - additional config variable
     - documentation changes
     - debate about what the default should be. GOTO 1.

I personally would be happy with any of these options, so I think we get
some more feedback to see if a consensus emerges. It's a tiny patch and
trivial to test, so we have time(ing) on our side.

Reply via email to