-
-
Notifications
You must be signed in to change notification settings - Fork 177
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
Upgrade snappy to 1.1.7 #522
Conversation
Lost one more line for some reason, strange. Shouldn't we move this to a Git Submodule now that I think about it? |
It's probably wise to do so now yes. But might not be completely straight forward. Needs some work to get paths right etc (e.g. |
24924d9
to
baff261
Compare
This is actually great because we don't need to push the whole C++ codebase to Just looking for a way to remove all these dependencies after a |
I'd like to add something like this to {
"scripts": {
"make": "git submodule update --init && node-gyp rebuild",
"install": "prebuild-install || npm run make && git submodule deinit .",
}
} (I chose This would be nice for users because they wouldn't end up with a few megabytes of useless C++ dependencies, but would require developers and CI to download all git submodules twice. Any ideas? Except for that, this PR is ready. Maybe I should work on that on another PR actually. |
Actually, I don't think we should do the |
That's a good point. Wouldn't work if installing on a system without At least this should make it easier to update upstream dependencies ( |
Yes, it's a huge improvement when it comes to maintenance :) We should do this with all code that's vendored, e.g. leveldb and rocksdb as well. |
@ralphtheninja I'm already on it. 😉 Just making sure that all versions are exactly the same and that I'm not overriding a patch or something. |
Okay, this one should be good to go. 👍 Maybe push a test release to check if everything works on |
Will test tomorrow. |
Hey @ralphtheninja, have you had some time to test this out? If so, I could start working on moving all dependencies to git modules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine, but I think we should document this in README for devs. Someone cloning the repo should probably clone it recursively (or do the git submodule dance)
Nice! |
Fixes #521