You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by brahmareddybattula <gi...@git.apache.org> on 2017/06/23 07:01:35 UTC
[GitHub] zookeeper pull request #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
GitHub user brahmareddybattula opened a pull request:
https://github.com/apache/zookeeper/pull/291
ZOOKEEPER-2818. Improve the ZooKeeper#setACL java doc
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/brahmareddybattula/zookeeper ZOOKEEPER-2818
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/291.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 #291
----
commit 6ece1bdb39694946dae2f03f74547b880c1d7769
Author: b00902108 <br...@huawei.com>
Date: 2017-06-23T06:59:10Z
ZOOKEEPER-2818. Improve the ZooKeeper#setACL java doc
----
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r123910526
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
I think it is worth to add a little bit description here - something like: `the acl version of the node`. I know, the name already encodes the semantic but being explicit is always better.
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r125140546
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
Thanks for updating. Yes no need patch every parameter in this patch.
>> given aclVersion matches the aclVersion of the node.
How about this: `given aclVersion matches the acl version of the node`. Similarly for the param:
>> aclVersion the given aclVersion for the node
`aclVersion - the given acl version of the node`.
Copy pasting does not make it more descriptive, thus my suggestion.
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL java doc
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/291
Thanks for update, @brahmareddybattula. Merged to master, 3.5, and 3.4.
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL java doc
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/291
@brahmareddybattula yes you should be added to contributor list. I think @rakeshadr could help get you added to the contributor list and assign ZOOKEEPER-2818 to you. I don't have JIRA admin permission yet.
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by brahmareddybattula <gi...@git.apache.org>.
Github user brahmareddybattula commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r124241306
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
ok..done.. But there are so many (example : org.apache.zookeeper.ZooKeeper#getChildren(java.lang.String, boolean, Stat)) not updated the params..
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/291
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL java doc
Posted by brahmareddybattula <gi...@git.apache.org>.
Github user brahmareddybattula commented on the issue:
https://github.com/apache/zookeeper/pull/291
thanks a lot for review and commit. looks I can't assign this issue to me, can you assign to me Or give contributor access..?
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by brahmareddybattula <gi...@git.apache.org>.
Github user brahmareddybattula commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r125200854
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
updated as per your suggestion.
thanks a lot for taking a look
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r124148111
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
I think we can add documentation to the other parameters as part of this JIRA as the additional work required does not justify creating another JIRA which brings overheads for both of us.
---
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 #291: ZOOKEEPER-2818. Improve the ZooKeeper#setACL ja...
Posted by brahmareddybattula <gi...@git.apache.org>.
Github user brahmareddybattula commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/291#discussion_r123914814
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -2347,25 +2347,25 @@ public void getACL(final String path, Stat stat, ACLCallback cb,
/**
* Set the ACL for the node of the given path if such a node exists and the
- * given version matches the version of the node. Return the stat of the
+ * given aclVersion matches the aclVersion of the node. Return the stat of the
* node.
* <p>
* A KeeperException with error code KeeperException.NoNode will be thrown
* if no node with the given path exists.
* <p>
* A KeeperException with error code KeeperException.BadVersion will be
- * thrown if the given version does not match the node's version.
+ * thrown if the given aclVersion does not match the node's aclVersion.
*
* @param path
* @param acl
- * @param version
+ * @param aclVersion
--- End diff --
In that case dn't we need to consider remain two args(path,acl) also..? if you agree, might be handle in another jira..?
---
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.
---