pinojs / pino-elasticsearch

🌲 load pino logs into Elasticsearch
MIT License
179 stars 67 forks source link

fix support for elasticsearch v7 + add types #169

Closed robyte-ctrl closed 1 year ago

robyte-ctrl commented 1 year ago

opened the PR in here by mistake (meant to merge it in my fork's master branch) so feel free to close it

but this PR does fix https://github.com/pinojs/pino-elasticsearch/issues/166 and it also adds types so it's easier to use this library in a typescript project

robyte-ctrl commented 1 year ago

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

mcollina commented 1 year ago

I see you have removed --bulk-size from the CLI options, hence my guess.

robyte-ctrl commented 1 year ago

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

image

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?

mcollina commented 1 year ago

ah no worries! That's ok.