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

⚡️ V2 Rewrite — Get rid of lz4 dependency #7

Merged
merged 8 commits into from
Dec 4, 2020
Merged

Conversation

paambaati
Copy link
Collaborator

The lz4 package has installation issues, and seems to be not maintained anymore.

This PR aims to remove this dependency entirely and instead use lz4-asm. It is actively maintained and offers near-native performance.

This PR should fix #5 and #3

Other changes include —

  1. Minimum required Node.js version now become 10 (because of lz4-asm).
  2. Travis CI configuration is removed in favour of Github Actions.
  3. TSLint is replaced with ESLint + Prettier for a more modern and consistent style.
  4. Test suite now runs against Kafka v2.5.0.
  5. All dependencies upgraded to their latest stable versions.

Rollout strategy

We'll be publishing this as a a beta (yarn add [email protected]), collect feedback and then promote 2.0.0 to stable after a few weeks.

@Prsna23 Can you also upgrade to this internally and ensure this is stable?

CC: @ankon @Freakachoo

@paambaati paambaati self-assigned this Dec 4, 2020
@paambaati paambaati added the enhancement New feature or request label Dec 4, 2020
@paambaati
Copy link
Collaborator Author

paambaati commented Dec 4, 2020

I've just published this to NPM. This is now available to install with —

@Prsna23 @ankon @Freakachoo Can you folks let me know if this beta version works for you?

@paambaati paambaati merged commit 023de1b into master Dec 4, 2020
@paambaati paambaati deleted the v2-rewrite branch December 4, 2020 08:48
@Prsna23
Copy link
Contributor

Prsna23 commented Dec 4, 2020

@paambaati I don't think we will be upgrading it for a while now. I am able to pull this package without a problem.

src/index.ts Show resolved Hide resolved
@ankon
Copy link
Contributor

ankon commented Dec 4, 2020

We replaced kafkajs-lz4 with a simple trivial wrapper based on #2. I'll have to check whether switching to lz4-asm would be worth the effort for us, but it's definitely nice to see activity here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn add kafkajs-lz4 issue
3 participants