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

Updated thrift version. #47

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gareth-robinson
Copy link

The older verson of thrift used by this library was using a version of thrift that had 'nodeunit' as a dependency, when it should have been a devDependency. This meant that a lot of test libraries ended up bloating production builds using this library.

I also made a few other minor changes

  • removed some of the blank dependency versions as they appeared to cause 'npm audit' to fail
  • set the license field
  • removed the bin/tester file and just put the command directly into package.json
  • added the package-lock.json

Ran the unit tests and linter and they all passed.

…(not devDependency), meaning that a lot of test libraries ended up bloating production builds using this library
@gareth-robinson
Copy link
Author

Hmm, ok, not so easy as I hoped there. It works locally, but I'll see if I can work out what the travis issue is.

…oc, only introduced in node 5.10. Updated the travis and package engines to start testing from v6
@gareth-robinson
Copy link
Author

The latest version of thrift used a node method Buffer.alloc, which didn't exist before v5.10 so in changing the thrift version I also had to change the target 'engines' for this library to v6 and above - also changed the travis package to match.

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.

1 participant