-
Notifications
You must be signed in to change notification settings - Fork 67
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
fix support for elasticsearch v7 + add types #169
fix support for elasticsearch v7 + add types #169
Conversation
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.
Good work!
This is a semver-major change, right?
@mcollina shouldn't be, although I have renamed some of the options so that they follow the camelCase naming notation, the old names still work and I've added some tests to make sure that they do if you want to remove the old names completely then that would be a semver-major change |
I see you have removed --bulk-size from the CLI options, hence my guess. |
@mcollina oh, right, I removed it because it wasn't doing anything not sure how that could be considered deprecated since a deprecation usually means that the deprecated thing still works have I perhaps missed something? should I add it back and leave it as is just to keep the interface the same? |
ah no worries! That's ok. |
ef6e8ff
to
3bf6a7b
Compare
opened the PR in here by mistake (meant to merge it in my fork's
master
branch) so feel free to close itbut this PR does fix #166 and it also adds types so it's easier to use this library in a typescript project