Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ZooKeeper C Client v3.5.8 in all platforms #260

Merged
merged 5 commits into from
Dec 26, 2020

Conversation

DavidVujic
Copy link
Collaborator

@DavidVujic DavidVujic commented Dec 25, 2020

Description

When installing this package on Windows, an old version of the C client (3.4.12) is used because of issues related to #222

With v3.5.8 used in this PR, the issues for Windows are resolved and it is possible to build an addon from the same source code.

In addition to that, the no longer needed install scripts are removed and new prebuilds for Mac OS X and Windows are packaged.

One more thing: the ZooKeeper source code is slimmed down to only including the C client code. The package has been pre-configured using ant and autoreconf - so there is no need for the patching (copying of autoconf generated files in the patches folder) that previously had to be done. This will result in a much lighter npm package (currently 5 MB). 🎉

Motivation and Context

With this setup, adding features will be a whole lot easier.

How Has This Been Tested?

Travis CI build OK
Tested manually by running a local Zookeeper server and start a client by running node examples/index.js in Linux, Mac OS X and Windows 10.

Tested with ZooKeeper server 3.4.8 (the one that was causing issues) and the latest version.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

@DavidVujic
Copy link
Collaborator Author

With this setup, adding features will be a whole lot easier. When this one is merged, this PR will no longer need to separately handle Windows.

@DavidVujic DavidVujic merged commit 9bbc2c8 into yfinkelstein:master Dec 26, 2020
@DavidVujic DavidVujic deleted the 3-5-8-for-all-platforms branch December 26, 2020 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants