kezhuw commented on PR #2267:
URL: https://github.com/apache/zookeeper/pull/2267#issuecomment-2995212466

   > breaking changes are not advised in a subminor version. I totally agree 
that, shutdown() is not an API, but this change stops from adoption immediately.
   
   
[ZooKeeperServerEmbedded](https://zookeeper.apache.org/doc/r3.9.3/apidocs/zookeeper-server/org/apache/zookeeper/server/embedded/ZooKeeperServerEmbedded.html)
 was designed to be public in mind. Curator has migrated to it but it still 
have source code level dependencies on `ZooKeeperServer`.
   
   > There was no justification provided while making it final in the commit.
   
   My cents:
   1. `void shutdown()` was changed to `final` to fix ZOOKEEPER-4712.
   2. I would like `void shutdown(boolean fullyShutDown)` to be internal usages.
   3. `void shutdownComponents()` should be used instead above two.
   
   > Change https://github.com/apache/curator/pull/1252 would solve the 
problem. But that needs a release from Curator and down streams needs to adopt 
that as well which are not as frequent as zookeeper.
   
   This is a compilation error, people will have to upgrade either curator or 
zookeeper anyway. I started a discuss to release a curator version for this: 
https://lists.apache.org/thread/jywgcrk11whh8km7q6btc6k574vx1lrq
   
   Given above, it would be -0 from my side to make `void shutdown(boolean 
fullyShutDown)` non final.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to