ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-731

Zookeeper#delete , #create - async versions miss a verb in the javadoc

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.3.0
    • Fix Version/s: 3.4.0
    • Component/s: documentation
    • Labels:
      None

      Description

      /**

      • The Asynchronous version of delete. "The request doesn't missing actually until
      • the asynchronous callback is called."
        */
        public void delete(final String path, int version, VoidCallback cb, Object ctx) ..

      Also some information in the javadoc about how to instantiate the callback objects / context would be useful .

      1. ZOOKEEPER-731-fix2.patch
        0.8 kB
        Camille Fournier
      2. ZOOKEEPER-731-fix.patch
        0.7 kB
        Camille Fournier
      3. ZOOKEEPER-731.patch
        6 kB
        Thomas Koch

        Activity

        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1296 (See https://builds.apache.org/job/ZooKeeper-trunk/1296/)
        ZOOKEEPER-731. Zookeeper#delete , #create - async versions miss a verb in the javadoc (Thomas Koch via camille)
        ZOOKEEPER-731. Zookeeper#delete , #create - async versions miss a verb in the javadoc (Thomas Koch via camille)

        camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1165443
        Files :

        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java

        camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1165406
        Files :

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1296 (See https://builds.apache.org/job/ZooKeeper-trunk/1296/ ) ZOOKEEPER-731 . Zookeeper#delete , #create - async versions miss a verb in the javadoc (Thomas Koch via camille) ZOOKEEPER-731 . Zookeeper#delete , #create - async versions miss a verb in the javadoc (Thomas Koch via camille) camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1165443 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java camille : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1165406 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12493080/ZOOKEEPER-731-fix2.patch
        against trunk revision 1165443.

        +1 @author. The patch does not contain any @author tags.

        +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/504//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12493080/ZOOKEEPER-731-fix2.patch against trunk revision 1165443. +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/504//console This message is automatically generated.
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-05 20:04:12, Patrick Hunt wrote:

        > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94

        > <https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90>

        >

        > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document.

        Camille Fournier wrote:

        Pat,

        I checked this into head already. Would you prefer I roll back this part of the change or just add a comment?

        Patrick Hunt wrote:

        imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review...

        Camille Fournier wrote:

        Sounds good, will do.

        Cool.

        Btw, I do agree with Thomas this is a code smell - we are trying to output the environment detail once, when the first ZK client is instantiated. Perhaps this code just needs more heavy refactoring, i.e. rather than hanging logEnv in the static perhaps there's a better way?

        • Patrick

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1753
        -----------------------------------------------------------

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-05 20:04:12, Patrick Hunt wrote: > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94 > < https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90 > > > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document. Camille Fournier wrote: Pat, I checked this into head already. Would you prefer I roll back this part of the change or just add a comment? Patrick Hunt wrote: imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review... Camille Fournier wrote: Sounds good, will do. Cool. Btw, I do agree with Thomas this is a code smell - we are trying to output the environment detail once, when the first ZK client is instantiated. Perhaps this code just needs more heavy refactoring, i.e. rather than hanging logEnv in the static perhaps there's a better way? Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1753 ----------------------------------------------------------- On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-05 20:04:12, Patrick Hunt wrote:

        > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94

        > <https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90>

        >

        > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document.

        Camille Fournier wrote:

        Pat,

        I checked this into head already. Would you prefer I roll back this part of the change or just add a comment?

        Patrick Hunt wrote:

        imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review...

        Sounds good, will do.

        • Camille

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1753
        -----------------------------------------------------------

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-05 20:04:12, Patrick Hunt wrote: > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94 > < https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90 > > > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document. Camille Fournier wrote: Pat, I checked this into head already. Would you prefer I roll back this part of the change or just add a comment? Patrick Hunt wrote: imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review... Sounds good, will do. Camille ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1753 ----------------------------------------------------------- On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-05 20:04:12, Patrick Hunt wrote:

        > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94

        > <https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90>

        >

        > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document.

        Camille Fournier wrote:

        Pat,

        I checked this into head already. Would you prefer I roll back this part of the change or just add a comment?

        imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review...

        • Patrick

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1753
        -----------------------------------------------------------

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-05 20:04:12, Patrick Hunt wrote: > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94 > < https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90 > > > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document. Camille Fournier wrote: Pat, I checked this into head already. Would you prefer I roll back this part of the change or just add a comment? imo putting the initialization of LOG back into the static would be the right thing to do, but feel free to just add the comment. This is a small/discussed change, if you want you could a) just submit a second patch to the jira that moves the init back into the static and adds a comment on why we do it this way, then go ahead and commit that change. I don't believe we'd need to go through a more lengthy review... Patrick ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1753 ----------------------------------------------------------- On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-05 20:04:12, Patrick Hunt wrote:

        > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94

        > <https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90>

        >

        > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document.

        Pat,
        I checked this into head already. Would you prefer I roll back this part of the change or just add a comment?

        • Camille

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1753
        -----------------------------------------------------------

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-05 20:04:12, Patrick Hunt wrote: > src/java/main/org/apache/zookeeper/ZooKeeper.java, lines 90-94 > < https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90 > > > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document. Pat, I checked this into head already. Would you prefer I roll back this part of the change or just add a comment? Camille ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1753 ----------------------------------------------------------- On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1753
        -----------------------------------------------------------

        src/java/main/org/apache/zookeeper/ZooKeeper.java
        <https://reviews.apache.org/r/1715/#comment3983>

        I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document.

        • Patrick

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1753 ----------------------------------------------------------- src/java/main/org/apache/zookeeper/ZooKeeper.java < https://reviews.apache.org/r/1715/#comment3983 > I suspect LOG is initialized in the static to make initialization explicit - given logEnv uses LOG as an argument. We'll lose this with this change, i.e. someone might re-refactor the code and lose the implicit ordering. At the very least it would be a good idea to document. Patrick On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2011-09-05 19:43:32, Thomas Koch wrote:

        > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 90

        > <https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90>

        >

        > it is unrelated to the rest of the patch, but I touched the file anyway and it's an obvious clean up that follows the boy scout rule.[1]

        >

        > The previous LOG initialization was very confusing. It made me think whether there's some JVM magic that would require LOG to be initialized in the static block rather then in the normal way.

        >

        > [1] http://pragmaticcraftsman.com/2011/03/the-boy-scout-rule/

        Ok, seems reasonable. +1, I'll check this in to trunk.

        • Camille

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1751
        -----------------------------------------------------------

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - On 2011-09-05 19:43:32, Thomas Koch wrote: > src/java/main/org/apache/zookeeper/ZooKeeper.java, line 90 > < https://reviews.apache.org/r/1715/diff/1/?file=37951#file37951line90 > > > it is unrelated to the rest of the patch, but I touched the file anyway and it's an obvious clean up that follows the boy scout rule. [1] > > The previous LOG initialization was very confusing. It made me think whether there's some JVM magic that would require LOG to be initialized in the static block rather then in the normal way. > > [1] http://pragmaticcraftsman.com/2011/03/the-boy-scout-rule/ Ok, seems reasonable. +1, I'll check this in to trunk. Camille ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1751 ----------------------------------------------------------- On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1751
        -----------------------------------------------------------

        src/java/main/org/apache/zookeeper/ZooKeeper.java
        <https://reviews.apache.org/r/1715/#comment3981>

        it is unrelated to the rest of the patch, but I touched the file anyway and it's an obvious clean up that follows the boy scout rule.[1]

        The previous LOG initialization was very confusing. It made me think whether there's some JVM magic that would require LOG to be initialized in the static block rather then in the normal way.

        [1] http://pragmaticcraftsman.com/2011/03/the-boy-scout-rule/

        • Thomas

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1751 ----------------------------------------------------------- src/java/main/org/apache/zookeeper/ZooKeeper.java < https://reviews.apache.org/r/1715/#comment3981 > it is unrelated to the rest of the patch, but I touched the file anyway and it's an obvious clean up that follows the boy scout rule. [1] The previous LOG initialization was very confusing. It made me think whether there's some JVM magic that would require LOG to be initialized in the static block rather then in the normal way. [1] http://pragmaticcraftsman.com/2011/03/the-boy-scout-rule/ Thomas On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/#review1750
        -----------------------------------------------------------

        src/java/main/org/apache/zookeeper/ZooKeeper.java
        <https://reviews.apache.org/r/1715/#comment3980>

        Is there some reason you moved this LOG initialization as part of this patch? I don't think there's anything wrong with it but it does seem unrelated.

        • Camille

        On 2011-09-05 15:55:47, Thomas Koch wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/1715/

        -----------------------------------------------------------

        (Updated 2011-09-05 15:55:47)

        Review request for zookeeper.

        Summary

        -------

        .

        This addresses bug ZOOKEEPER-731.

        https://issues.apache.org/jira/browse/ZOOKEEPER-731

        Diffs

        -----

        src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f

        Diff: https://reviews.apache.org/r/1715/diff

        Testing

        -------

        Thanks,

        Thomas

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/#review1750 ----------------------------------------------------------- src/java/main/org/apache/zookeeper/ZooKeeper.java < https://reviews.apache.org/r/1715/#comment3980 > Is there some reason you moved this LOG initialization as part of this patch? I don't think there's anything wrong with it but it does seem unrelated. Camille On 2011-09-05 15:55:47, Thomas Koch wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- (Updated 2011-09-05 15:55:47) Review request for zookeeper. Summary ------- . This addresses bug ZOOKEEPER-731 . https://issues.apache.org/jira/browse/ZOOKEEPER-731 Diffs ----- src/java/main/org/apache/zookeeper/ZooKeeper.java 00bac9f Diff: https://reviews.apache.org/r/1715/diff Testing ------- Thanks, Thomas
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12493037/ZOOKEEPER-731.patch
        against trunk revision 1164758.

        +1 @author. The patch does not contain any @author tags.

        +0 tests included. The patch appears to be a documentation patch that doesn't require tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12493037/ZOOKEEPER-731.patch against trunk revision 1164758. +1 @author. The patch does not contain any @author tags. +0 tests included. The patch appears to be a documentation patch that doesn't require tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/501//console This message is automatically generated.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/1715/
        -----------------------------------------------------------

        Review request for zookeeper.

        Summary
        -------

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1715/ ----------------------------------------------------------- Review request for zookeeper. Summary -------
        Hide
        Mahadev konar added a comment -

        moving this to 3.4... not a regression for 3.3.

        Show
        Mahadev konar added a comment - moving this to 3.4... not a regression for 3.3.

          People

          • Assignee:
            Thomas Koch
            Reporter:
            Karthik K
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development