You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/06/11 11:13:23 UTC
[GitHub] [activemq-artemis] jsmucr opened a new pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
jsmucr opened a new pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177
HumanReadableByteCountTest test is no longer failing under environments with locales defining different number format.
The function now returns values according to the Locale.ROOT locale specification.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642820835
Doesn't this mean that the "human readable format" won't be formatted according to the JVM's default locale? It seems like this is the opposite of what we want. Wouldn't we want to change the *test* so that it formats the assertion value according to the default locale?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on a change in pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on a change in pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#discussion_r438853388
##########
File path: artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ByteUtil.java
##########
@@ -449,6 +450,7 @@ public static String getHumanReadableByteCount(long bytes) {
i++;
}
- return String.format("%.1f%sB", bytes / BYTE_MAGNITUDES[i], BYTE_SUFFIXES[i]);
+ // Set locale to language/country neutral to avoid different behavior for non-US users
Review comment:
yes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-655804174
beware that I merged this by accident, then I reverted it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#discussion_r438850048
##########
File path: artemis-commons/src/main/java/org/apache/activemq/artemis/utils/ByteUtil.java
##########
@@ -449,6 +450,7 @@ public static String getHumanReadableByteCount(long bytes) {
i++;
}
- return String.format("%.1f%sB", bytes / BYTE_MAGNITUDES[i], BYTE_SUFFIXES[i]);
+ // Set locale to language/country neutral to avoid different behavior for non-US users
Review comment:
Nice!!! @ehsavoie you posted about this on the dev list the other day, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642861852
@ehsavoie, I just responded to your email on the dev list.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642852520
@jbertram that was the point I tried to raise in my email. What do we want ? ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-655598655
@jsmucr I think should be closed, as the test on #3178 would fail.
If you want to still discuss this, please reopen the PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642852520
@jbertram that was the point I tried to raise in my email. What do we want ? ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jsmucr edited a comment on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
jsmucr edited a comment on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-643057629
@jbertram I too thought about that but for me it seemed a bit tough and not very transparent to cover various locales in a test. What I mean is that you'd either have to enumerate the possible values, or compare the method return value to something you cannot really see at the moment of testing.
Maybe I'm not fully awake yet but wouldn't that mean to pretty much copy-paste the method functionality to the test?
EDIT: I see what you did in #3178. It avoids the other differences such as grouping which is ok as long as it returns kB instead of KiB. Is that correct? I mean, I know the difference. But is that what the regular user would expect?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] ehsavoie commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
ehsavoie commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642852520
@jbertram that was the point I tried to raise in my email. What do we want ? ;)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic closed pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
clebertsuconic closed pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-642827673
I sent #3178 to demonstrate what I mean.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jsmucr commented on pull request #3177: ARTEMIS-2801 Fix ByteUtil.getHumanReadableByteCount() giving inconsistent results
Posted by GitBox <gi...@apache.org>.
jsmucr commented on pull request #3177:
URL: https://github.com/apache/activemq-artemis/pull/3177#issuecomment-643057629
@jbertram I too thought about that but for me it seemed a bit tough and not very transparent to cover various locales in a test. What I mean is that you'd either have to enumerate the possible values, or compare the method return value to something you cannot really see at the moment of testing.
Maybe I'm not fully awake yet but wouldn't that mean to pretty much copy-paste the method functionality to the test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org