[ https://issues.apache.org/jira/browse/HTTPCORE-611?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16962344#comment-16962344 ]
Gary D. Gregory edited comment on HTTPCORE-611 at 10/29/19 7:38 PM: -------------------------------------------------------------------- Ok, so in master we have the following done: * *{{NEG_ONE_*SECONDS}} uses plural for the scale of one, but should use singular* * *{{defaultsToZeroMillis}} is not consistent with {{defaultsToNegativeOneSecond}}. It should be {{defaultsToZeroMilliseconds}}* * *{{parse()}} uses {{String.format()}} but never passes value to the format. {{%s}} is missing.* * *{{toMillis()}} and other second fractions use inconsistent naming. I'd expect it to be {{toMilliseconds()}} and so forth.* And here are my comments on the remainder: * *{{toString()}} should also do at most {{%d}} or {{%s}}.* I disagree, using `%,d` is much easier to read and is localized. The Javadoc should specify the string is localized. Maybe we need a toString() variant that takes a boolean to localize or not. Java's time package provided classes that "Local" * *{{parse()}} should parse with {{Long.parseLong()}} instead of a biased locale. 1,200 SECONDS for me is one second and 200 milliseconds. Thus, it should be language agnostic.* I disagree, I want users to be able to use "1,200 SECONDS" in configuration files in their locale and have their expectations met. If you want to be language agnostic, don't use a separator. Simple. The Javadoc should be improved. was (Author: garydgregory): Ok, so in master we have the following done: * *{{NEG_ONE_*SECONDS}} uses plural for the scale of one, but should use singular* * *{{defaultsToZeroMillis}} is not consistent with {{defaultsToNegativeOneSecond}}. It should be {{defaultsToZeroMilliseconds}}* * *{{parse()}} uses {{String.format()}} but never passes value to the format. {{%s}} is missing.* * *{{toMillis()}} and other second fractions use inconsistent naming. I'd expect it to be {{toMilliseconds()}} and so forth.* And here are my comments on the remainder: * *{{toString()}} should also do at most {{%d}} or {{%s}}.* I disagree, using `%,d` is much easier to read and is localized. The Javadoc should specify the string is localized. Java's time package provided classes that "Local" * *{{parse()}} should parse with {{Long.parseLong()}} instead of a biased locale. 1,200 SECONDS for me is one second and 200 milliseconds. Thus, it should be language agnostic.* I disagree, I want users to be able to use "1,200 SECONDS" in configuration files in their locale and have their expectations met. If you want to be language agnostic, don't use a separator. Simple. The Javadoc should be improved. > Minor glitches with TimeValue > ----------------------------- > > Key: HTTPCORE-611 > URL: https://issues.apache.org/jira/browse/HTTPCORE-611 > Project: HttpComponents HttpCore > Issue Type: Improvement > Components: HttpCore > Affects Versions: 5.0-beta9 > Reporter: Michael Osipov > Assignee: Gary D. Gregory > Priority: Major > Fix For: 5.0-beta11 > > Time Spent: 20m > Remaining Estimate: 0h > > The {{TimeValue}} looks like a very good thing. Some nits can be improved: > * {{NEG_ONE_*SECONDS}} uses plural for the scale of one, but should use > singular > * {{defaultsToZeroMillis}} is not consistent with > {{defaultsToNegativeOneSecond}}. It should be {{defaultsToZeroMilliseconds}} > * {{parse()}} uses {{String.format()}} but never passes value to the format. > {{%s}} is missing. > * {{parse()}} should parse with {{Long.parseLong()}} instead of a biased > locale. 1,200 SECONDS for me is one second and 200 milliseconds. Thus, it > should be language agnostic. > * {{toString()}} should also do at most {{%d}} or {{%s}}. > * {{toMillis()}} and other second fractions use inconsistent naming. I'd > expect it to be {{toMilliseconds()}} and so forth. > [~ggregory] -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org For additional commands, e-mail: dev-h...@hc.apache.org