You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by nikhilbhide <gi...@git.apache.org> on 2017/08/24 10:44:36 UTC
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
GitHub user nikhilbhide opened a pull request:
https://github.com/apache/zookeeper/pull/345
Zookeeper 2814: ignore space after comma in connection string
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nikhilbhide/zookeeper ZOOKEEPER-2814-Ignore-space-after-comma-in-connection-string
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #345
----
commit a533e730e70788046d4ab5869702e0ca4a70c5d8
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:15:19Z
Create StringUtils.java
Class which provides common String utilities
commit 80063abce26cd9a392ff1b1d6b9b323200241832
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:20:51Z
ZOOKEEPER-2814: Ignore space after comma in connection string
commit 9a4b65e88f091046f2948219894b419721ad2945
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T06:32:33Z
Added test to check whether connection string with trims is allowed
commit 894d3a8ff295b81f94c14df913938c899bbcff63
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:46:57Z
Added code to test split logic implemented in StringUtils.java
commit 6b2de4ca53cce67ecdb1a714ec9ad29a442bfd20
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:55:25Z
Added import of java.util.List
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #345: Zookeeper 2814: ignore space after comma in connection...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on the issue:
https://github.com/apache/zookeeper/pull/345
Patch is ready to be applied
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135077391
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
+
--- End diff --
nit: extra empty line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
GitHub user nikhilbhide reopened a pull request:
https://github.com/apache/zookeeper/pull/345
Zookeeper 2814: ignore space after comma in connection string
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nikhilbhide/zookeeper ZOOKEEPER-2814-Ignore-space-after-comma-in-connection-string
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #345
----
commit a533e730e70788046d4ab5869702e0ca4a70c5d8
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:15:19Z
Create StringUtils.java
Class which provides common String utilities
commit 80063abce26cd9a392ff1b1d6b9b323200241832
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:20:51Z
ZOOKEEPER-2814: Ignore space after comma in connection string
commit 9a4b65e88f091046f2948219894b419721ad2945
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T06:32:33Z
Added test to check whether connection string with trims is allowed
commit 894d3a8ff295b81f94c14df913938c899bbcff63
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:46:57Z
Added code to test split logic implemented in StringUtils.java
commit 6b2de4ca53cce67ecdb1a714ec9ad29a442bfd20
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:55:25Z
Added import of java.util.List
commit 93d72875bf5d97ab9bcea710e0d601d2fdeaf970
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T17:43:07Z
Removed extra space from ConnectStringParser.java
commit 37ec5e92a73358d116c5475e07872fb9c35d1aa2
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T17:49:21Z
Update StringUtils.java
commit a446b2bceacb96aa7e3d2b9e4639c3919f5d31e5
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T17:50:34Z
Removed unnecessary usage of toString in StringUtilTest#testStrings
commit 03790ea7d5070b6f8961890b8bc865b8c433d9e9
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T17:51:15Z
Removed extra space after package declaration in StringUtilTest.java
commit 09dbd3d368ce1c62a3f0d1dcd98623b12bca6f3a
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T17:53:59Z
Added code to convert java.util.Collections$UnmodifiableRandomAccessList to string
----
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r136246013
--- Diff: src/java/main/org/apache/zookeeper/common/StringUtils.java ---
@@ -0,0 +1,44 @@
+/* Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
--- End diff --
Hi afine: Can you please complete the review comments? Can you please approve the changes so that patch can be created.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135077838
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
--- End diff --
nit: I could be wrong on this, but I think in general we prefer to import classes and not static methods directly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide closed the pull request at:
https://github.com/apache/zookeeper/pull/345
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135085577
--- Diff: src/java/main/org/apache/zookeeper/common/StringUtils.java ---
@@ -0,0 +1,44 @@
+/* Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
--- End diff --
afine: added required space. please review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135085831
--- Diff: src/java/test/org/apache/zookeeper/test/StringUtilTest.java ---
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.test;
+
+
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.common.StringUtils;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+public class StringUtilTest extends ZKTestCase {
+
+ @Test
+ public void testStrings() {
+
+ String s1 = " a , b , ";
+ assertEquals("[a, b]", StringUtils.split(s1, ",").toString());
--- End diff --
afine: Conversion to string is required as output of StringUtils.split returns java.util.Collections$UnmodifiableRandomAccessList<[a, b]>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135181414
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
--- End diff --
eribeiro: I have kept the code same, in master and branch-3.5 static method member is imported i.e import static org.apache.zookeeper.common.StringUtils.split
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide closed the pull request at:
https://github.com/apache/zookeeper/pull/345
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135078674
--- Diff: src/java/test/org/apache/zookeeper/test/StringUtilTest.java ---
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.test;
+
+
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.common.StringUtils;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+public class StringUtilTest extends ZKTestCase {
+
+ @Test
+ public void testStrings() {
+
+ String s1 = " a , b , ";
+ assertEquals("[a, b]", StringUtils.split(s1, ",").toString());
--- End diff --
I'm wondering if it would be possible to compare lists themselves rather than comparing the output of toString?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135083779
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
--- End diff --
Static import of individual member is allowed as it sometimes improves the readability
Please let me know your opinion on this and if required I will change it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
GitHub user nikhilbhide reopened a pull request:
https://github.com/apache/zookeeper/pull/345
Zookeeper 2814: ignore space after comma in connection string
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nikhilbhide/zookeeper ZOOKEEPER-2814-Ignore-space-after-comma-in-connection-string
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #345
----
commit a533e730e70788046d4ab5869702e0ca4a70c5d8
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:15:19Z
Create StringUtils.java
Class which provides common String utilities
commit 80063abce26cd9a392ff1b1d6b9b323200241832
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-23T18:20:51Z
ZOOKEEPER-2814: Ignore space after comma in connection string
commit 9a4b65e88f091046f2948219894b419721ad2945
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T06:32:33Z
Added test to check whether connection string with trims is allowed
commit 894d3a8ff295b81f94c14df913938c899bbcff63
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:46:57Z
Added code to test split logic implemented in StringUtils.java
commit 6b2de4ca53cce67ecdb1a714ec9ad29a442bfd20
Author: Nikhil Bhide <ni...@gmail.com>
Date: 2017-08-24T09:55:25Z
Added import of java.util.List
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135106003
--- Diff: src/java/test/org/apache/zookeeper/test/StringUtilTest.java ---
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.test;
+
+
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.common.StringUtils;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+public class StringUtilTest extends ZKTestCase {
+
+ @Test
+ public void testStrings() {
+
+ String s1 = " a , b , ";
+ assertEquals("[a, b]", StringUtils.split(s1, ",").toString());
--- End diff --
What is wrong with using
```
String s1 = " a , b , ";
List<String> expected = Arrays.asList("a", "b");
assertTrue(expected.equals(StringUtils.split(s1, ",")));
```
????
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135078033
--- Diff: src/java/main/org/apache/zookeeper/common/StringUtils.java ---
@@ -0,0 +1,44 @@
+/* Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.common;
--- End diff --
nit: add a space here
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135084898
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
+
--- End diff --
afine: Removed it. please review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #345: Zookeeper 2814: ignore space after comma in connection...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on the issue:
https://github.com/apache/zookeeper/pull/345
Patch is ready
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135207087
--- Diff: src/java/test/org/apache/zookeeper/test/StringUtilTest.java ---
@@ -0,0 +1,44 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.zookeeper.test;
+
+
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.common.StringUtils;
+import org.junit.Test;
+
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+public class StringUtilTest extends ZKTestCase {
+
+ @Test
+ public void testStrings() {
+
+ String s1 = " a , b , ";
+ assertEquals("[a, b]", StringUtils.split(s1, ",").toString());
--- End diff --
It works, and I can change the asserts. However, I have retrofitted the test from master and branch 3.5 so is it okay to change it in this particular branch? Please let me know.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r136245995
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
--- End diff --
eribeiro: Can you please complete the review comments? Can you please approve the changes so that patch can be created.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by eribeiro <gi...@git.apache.org>.
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/345#discussion_r135104691
--- Diff: src/java/main/org/apache/zookeeper/client/ConnectStringParser.java ---
@@ -20,8 +20,11 @@
import java.net.InetSocketAddress;
import java.util.ArrayList;
+import java.util.List;
import org.apache.zookeeper.common.PathUtils;
+import static org.apache.zookeeper.common.StringUtils.split;
--- End diff --
This code is similar to existing one in master and branch-3.5. For consistency, we could use what kind of import it uses, imo.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #345: Zookeeper 2814: ignore space after comma in con...
Posted by nikhilbhide <gi...@git.apache.org>.
Github user nikhilbhide closed the pull request at:
https://github.com/apache/zookeeper/pull/345
---