You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by tumativ <gi...@git.apache.org> on 2018/11/21 13:20:19 UTC
[GitHub] zookeeper pull request #712: Master FindBug issues fix
GitHub user tumativ opened a pull request:
https://github.com/apache/zookeeper/pull/712
Master FindBug issues fix
The fix to resolve the find bug issues introduced in master
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tumativ/zookeeper master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/712.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 #712
----
commit e117fc614bd781daa81d17a219188f5cd7428d6d
Author: vtumati <vt...@...>
Date: 2018-11-21T13:17:31Z
Master FindBug issues fix
----
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235384152
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
+ protected static final Logger LOG = LoggerFactory.getLogger(ZookeeperServerStabilizerConfig.class);
--- End diff --
Please use 4 spaces for indenting.
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235386518
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
+ protected static final Logger LOG = LoggerFactory.getLogger(ZookeeperServerStabilizerConfig.class);
--- End diff --
Sure.
---
[GitHub] zookeeper issue #712: Master FindBug issues fix
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/712
@tumativ Given that this is not a straight revert of the failing patch, would you please create a Jira and explain the reasoning in that?
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235386450
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
--- End diff --
Do we have to introduce a brand new class ? a simple variable won't be enough ?
---
[GitHub] zookeeper issue #712: Master FindBug issues fix
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on the issue:
https://github.com/apache/zookeeper/pull/712
Can we add in the descriptoin the original problem fro findbugs ?
It is not clear what we are going to fix.
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235384071
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
--- End diff --
What does this name supposed to mean?
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235388585
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
--- End diff --
The idea is to move all attributes which can be used to control the server. So that it can be managed and used in other places as well if requires
---
[GitHub] zookeeper pull request #712: Maintain the configuration will be used by serv...
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r237324365
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
+ protected static final Logger LOG = LoggerFactory.getLogger(ZookeeperServerStabilizerConfig.class);
--- End diff --
Done
---
[GitHub] zookeeper issue #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on the issue:
https://github.com/apache/zookeeper/pull/712
https://issues.apache.org/jira/browse/ZOOKEEPER-3196
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235391144
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
--- End diff --
Done
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235383973
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
--- End diff --
This comment should contain the Apache License header.
---
[GitHub] zookeeper pull request #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/712#discussion_r235395333
--- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/common/ZookeeperServerStabilizerConfig.java ---
@@ -0,0 +1,30 @@
+/**
+ *
+ */
+package org.apache.zookeeper.common;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ZookeeperServerStabilizerConfig {
--- End diff --
What does this name supposed to mean? The intention was that the configuration will be used by server stabilizer, so named ZookeeperServerStabilizerConfig
---
[GitHub] zookeeper issue #712: Master FindBug issues fix
Posted by tumativ <gi...@git.apache.org>.
Github user tumativ commented on the issue:
https://github.com/apache/zookeeper/pull/712
The build is green. It addressed the issues.
---