You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/05/01 00:55:23 UTC
[GitHub] [accumulo] hguercan opened a new pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
hguercan opened a new pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065
replace immutable HashSet usages by Set.of() to reduce code
* adjusted imports
* closes #2040
--
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] [accumulo] EdColeman commented on a change in pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
EdColeman commented on a change in pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#discussion_r624699445
##########
File path: core/src/test/java/org/apache/accumulo/core/util/NumUtilTest.java
##########
@@ -22,12 +22,15 @@
import static org.apache.accumulo.core.util.NumUtil.bigNumberForSize;
import static org.junit.Assert.assertEquals;
+import java.util.Locale;
+
import org.junit.Test;
public class NumUtilTest {
@Test
public void testBigNumberForSize() {
+ Locale.setDefault(Locale.US);
Review comment:
I created https://github.com/apache/accumulo/pull/2072 - that will separate this issue into its own commit. Once that change is merged, this can be re-based and merged.
--
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] [accumulo] hguercan commented on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
hguercan commented on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830646088
Also added Locale setting in NumUtilTest to US since unit testing fails if LC settings for numbers are set to DE for example
accumulo git:(replace_hashset_by_setof) ✗ find . -name "*.java" | xargs grep asList | grep new | grep Set
./core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java: Sets.copyOf but wasnt sure about null values
./test/src/main/java/org/apache/accumulo/test/UserCompactionStrategyIT.java: SortedSet --> TreeSet
./test/src/main/java/org/apache/accumulo/test/OrIteratorIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/OrIteratorIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/functional/ConfigurableCompactionIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/functional/SummaryIT.java: TreeSet
./shell/src/main/java/org/apache/accumulo/shell/commands/DUCommand.java: SortedSet --> TreeSet
./server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java: TreeSet
./server/manager/src/test/java/org/apache/accumulo/manager/replication/UnorderedWorkAssignerTest.java: gets modified
./server/base/src/main/java/org/apache/accumulo/server/rpc/TServerUtils.java: gets modified
--
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] [accumulo] EdColeman merged pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
EdColeman merged pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065
--
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] [accumulo] EdColeman commented on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830492935
Thanks! Was hoping to avoid work duplication if you had.
--
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] [accumulo] hguercan edited a comment on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
hguercan edited a comment on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830646088
Also added Locale setting in NumUtilTest to US since unit testing fails if LC settings for numbers are set to DE for example
accumulo git:(replace_hashset_by_setof) ✗ find . -name "*.java" | xargs grep asList | grep new | grep Set
./core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java: Sets.copyOf could have been used but wasnt sure about null values
./test/src/main/java/org/apache/accumulo/test/UserCompactionStrategyIT.java: SortedSet --> TreeSet
./test/src/main/java/org/apache/accumulo/test/OrIteratorIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/OrIteratorIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/functional/ConfigurableCompactionIT.java: TreeSet
./test/src/main/java/org/apache/accumulo/test/functional/SummaryIT.java: TreeSet
./shell/src/main/java/org/apache/accumulo/shell/commands/DUCommand.java: SortedSet --> TreeSet
./server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java: TreeSet
./server/manager/src/test/java/org/apache/accumulo/manager/replication/UnorderedWorkAssignerTest.java: gets modified
./server/base/src/main/java/org/apache/accumulo/server/rpc/TServerUtils.java: gets modified
--
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] [accumulo] ctubbsii commented on a change in pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#discussion_r624538274
##########
File path: core/src/test/java/org/apache/accumulo/core/util/NumUtilTest.java
##########
@@ -22,12 +22,15 @@
import static org.apache.accumulo.core.util.NumUtil.bigNumberForSize;
import static org.junit.Assert.assertEquals;
+import java.util.Locale;
+
import org.junit.Test;
public class NumUtilTest {
@Test
public void testBigNumberForSize() {
+ Locale.setDefault(Locale.US);
Review comment:
This change seems unrelated. Is it necessary? If it's an issue, it could be a separate 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] [accumulo] hguercan commented on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
hguercan commented on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830491507
Will check the other files too, probably was using your proposed command not on project root level
--
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] [accumulo] EdColeman commented on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830821626
Thanks for the contribution @hguercan and welcome to the Apache Accumulo community. If you would like to be listed as a contributor on Accumulo's people page please make a website PR for people.md. https://github.com/apache/accumulo-website/edit/main/pages/people.md
If you intend to make more contributions, please consider subscribing to the dev list and introducing yourself. Also, you can use the dev list or slack to reach out if you have questions about anything. https://accumulo.apache.org/contact-us/
--
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] [accumulo] hguercan commented on pull request #2065: #2040: replace immutable HashSet by Set.of() to reduce code
Posted by GitBox <gi...@apache.org>.
hguercan commented on pull request #2065:
URL: https://github.com/apache/accumulo/pull/2065#issuecomment-830695259
Thx for the reviews. Have to admit with the Locale.US addition I also was not sure if it belongs to "this" issue or should be addressed in a dedicated PR but thought unit tests needs to be independent as possible and this setting would be needed to get there and also to verify my work for this issue. Would be totally ok to revert and address it in a dedicated PR if you think differently.
--
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