[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-16 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-522197888 Thanks for your review @hanm ! @eolivelli I also gave it a second pass and reverted/addressed ugly formats

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521730229 Also there is a pain multiple log format which also make formatting harder. I would propose a best effort on the

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-15 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521729375 Line breaks, i.e., confusing line formats request changed above, suffer from lack usage of local variables, which

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-14 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521273094 I have redone the task enable checkstyle configuration and resolve conflict. Basically it focuses on whitespaces,

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-14 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-521270729 retest this please This is an automated message

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-13 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520765229 For keeping comments, I can proceed the work on this pull request. But it is unreasonable I send a fixup onto

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-13 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520701237 @eolivelli > All of the commits will be squashed into a single one. I am aware of this. In

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520634570 With read comments above, I'd like to propose group rules in different concerns into different issue/pull

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520605920 @hanm >There are lots of changes to exception types thrown by functions, where it changes concrete

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520376969 > Do we know whether the change in visibility to some of the methods will effect curator or other third parties?

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-12 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520330217 @nkalmar Thanks for your review and Thanks for @enixon 's comments. I would try to give a look and address them

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-11 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520221001 saved with the last fixup. If we want to merge this pr into master in multi commits, ping me to rebase the

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-11 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520220611 Fails on ``` [ERROR]

[GitHub] [zookeeper] TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server

2019-08-11 Thread GitBox
TisonKun commented on issue #1049: ZOOKEEPER-3475 Enable Checkstyle configuration on zookeeper-server URL: https://github.com/apache/zookeeper/pull/1049#issuecomment-520219117 Done on all packages under zookeeper-server. Please review if you have spare time ^_^