You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by afine <gi...@git.apache.org> on 2017/07/24 22:36:01 UTC
[GitHub] zookeeper pull request #316: ZOOKEEPER-2829: Interface usability / compatibi...
GitHub user afine opened a pull request:
https://github.com/apache/zookeeper/pull/316
ZOOKEEPER-2829: Interface usability / compatibility improvements through Java annotation.
This patch uses Apache Yetus audience annotations to label our publicly available interfaces and then generate our javadoc based on the annotations. The javadoc generated by this patch should be identical to our javadoc before with a few extra classes (that I think should have been included before anyway).
HostProvider
Record
StaticHostProvider
Transaction
ZKClientConfig
The "gotcha" with this patch is the way that java classes generated by jute are handled. There are four of these classes that need to be publicly documented: ACL, Id, Stat, StatPersisted (in addition to their superclass Record). I thought it would be safest to have the jute compiler always label these as "Public" and then we can filter out the ones we don't want in the javadoc ant task (by excluding the org.apache.zookeeper.server package and then pulling in the tools classes separately).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/afine/zookeeper ZOOKEEPER-2829
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/316.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 #316
----
commit c238a9f23787aade9122e7a9068b185d6c1fcf1b
Author: Abraham Fine <ab...@cloudera.com>
Date: 2017-07-24T22:11:43Z
ZOOKEEPER-2829: Interface usability / compatibility improvements through Java annotation.
----
---
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 #316: ZOOKEEPER-2829: Interface usability / compatibility im...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on the issue:
https://github.com/apache/zookeeper/pull/316
The only other change in the output of our javadoc due to this patch are extra packages appearing in the package list in the javadoc. Those packages do not include any "Public" classes and a user clicking on them will reveal no documented classes. This could be fixed by manually excluding those packages in the javadoc ant task but doing so would "defeat the purpose" of handling exclusion using the annotations. The behavior is consistent with other projects in our ecosystem such as Hadoop and HBase.
---
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 #316: ZOOKEEPER-2829: Interface usability / compatibility im...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/316
Sounds good to me - my concern was that these empty packages should be removed but since Hadoop and Hbase also has this "feature", and removing them default the purpose of the annotation, I am OK with current approach.
I also verified the new javadoc with "ant javadoc", the newly added classes / APIs also look good. Overall LGTM, good work @afine. Will merge soon.
---
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 #316: ZOOKEEPER-2829: Interface usability / compatibi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/316
---
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 #316: ZOOKEEPER-2829: Interface usability / compatibility im...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/316
>> The javadoc generated by this patch should be identical to our javadoc before with a few extra classes (that I think should have been included before anyway).
I suggest we scope this JIRA so it only focuses on the first part: "The javadoc generated by this patch should be identical to our javadoc before". The remaining part such as whether or not include jute and other new APIs can be discussed on dev list and done in a separate 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.
---
[GitHub] zookeeper issue #316: ZOOKEEPER-2829: Interface usability / compatibility im...
Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:
https://github.com/apache/zookeeper/pull/316
Yes please split - that would be easier to land current patch and I expect it will take some discussions to nail down the complete set of new APIs to be exposed.
---
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 #316: ZOOKEEPER-2829: Interface usability / compatibility im...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on the issue:
https://github.com/apache/zookeeper/pull/316
@hanm I am happy to split it up if you insist. My concern is that just adding the annotations to our "normal" java classes does not actually do much since it is technically incomplete.
I thought it would be a good idea to do the javadoc generation change here because it provides us a reasonably full proof way of verifying that every class that should be labeled public has been labeled public. Otherwise it would be rather tedious to make sure that we have labeled all of our classes appropriately.
---
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.
---