Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/548
ping @mjeelanimsft
Would you please create separate pull request for branch-3.5 and branch-3.4?
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
Committed to master branch only, because it conflicts with 3.5.
@mjeelanimsft Would you please create separate pull request for branch-3.5
and branch-3.4?
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
Jenkins is green and we got 2 approvals. Committing.
---
Github user breed commented on the issue:
https://github.com/apache/zookeeper/pull/548
+1 i'll kick jenkins
---
Github user mjeelanimsft commented on the issue:
https://github.com/apache/zookeeper/pull/548
Thanks for the feedback @maoling - I've incorporated the below changes.
@maoling @anmolnar - let me know what you think
- Assert actual host and port in unit tests for getHostAndPort
Github user mjeelanimsft commented on the issue:
https://github.com/apache/zookeeper/pull/548
@anmolnar Resolved the merge conflict. You can see the commit with the unit
test changes here -
https://github.com/apache/zookeeper/pull/548/commits/ff4704984203921071877bdee4efef47b8c2a706
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
@mjeelanimsft I keep trying to refresh the page, but again I can't see the
new tests.
Also the patch has some conflicts with the latest master, would you please
resolve them?
---
Github user mjeelanimsft commented on the issue:
https://github.com/apache/zookeeper/pull/548
Thanks Andor - I've added the unit test cases - For the one w/o [ - its
also an invalid case for this method as its needed to specify the port. I've
added that as one at the end, let me know
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
@mjeelanimsft Now I can see it, thanks.
I'd write 3 tests at the minimum to validate all if branches in the method:
With and w/o the `[` prefix and a third one for the invalid case: no
Github user mjeelanimsft commented on the issue:
https://github.com/apache/zookeeper/pull/548
Hi @anmolnar - I see it here - would you mind looking now?
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
@mjeelanimsft Sorry, I still can't see unit tests for `splitServerConfig`.
---
Github user mjeelanimsft commented on the issue:
https://github.com/apache/zookeeper/pull/548
Thanks @anmolnar & @maoling - I've added the Unit Test as well as the
JavaDoc for splitServerConfig() method
---
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/548
@mjeelanimsft Just to clarify my latest comment: I'd to see a new class
`ConfigUtilsTest` in which you explicitly verify the behaviour of
`splitServerConfig`. Thanks.
---
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/548
- It seems that zk server side supports ipv6 style like this:
`server.1=[2001:db8:1::242:ac11:2]:2888:3888`,but zk client side supports ipv6
like this:`2001:db8:1::242:ac11:2:2181`.we need unify
14 matches
Mail list logo
