zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] anmolnar commented on issue #1056: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
Date Wed, 21 Aug 2019 19:59:57 GMT
anmolnar commented on issue #1056: ZOOKEEPER-3495: fix SnapshotDigestTest to work with JDK12+
URL: https://github.com/apache/zookeeper/pull/1056#issuecomment-523625426
   > The DIGEST_VERSION is made to be static final to make sure it won't be changed in
code with things like setVersion.
   Would you please be a little bit more specific here? I don't see what code changes are
u trying to protect DIGEST_VERSION from and why would my proposal brake that? I see the opposite:
making it package-private is as safe as static final while it makes the code cleaner and testable
at the same time.
   > It seems to be safer and cleaner to me with the current code implementation...
   Why? Could you please be more specific?
   In my opinion using interfaces with instantiated objects are cleaner and more unit test
   > Also DigestCalculator is not only used in DataTree, it will also being used in PrepRequestProcessor

   No problem with that. It can still use the interface to be injected as constructor dependency
same way as `DataTree` should use it.
   I'm looking forward to your patch and see how unit testing is carried out. I really want
to avoid reflection everywhere, because we can clearly see now how fragile it could be. Additionally
your proposal about preparing suitable snapshot files could be working, but that also breaks
unit testing principles.
   Let's see @symat 's updated patch and talk about which way to go forward.

This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

With regards,
Apache Git Services

View raw message