pkallos / winston-firehose

NodeJS module, winston logging transport which writes to AWS Firehose.
MIT License
13 stars 9 forks source link

Upgrade to Winston 3 #15

Closed acinader closed 5 years ago

acinader commented 6 years ago
  1. Use node 10
  2. Make tests pass with Winston 3
  3. Rip out babel
    • was only being used for 'new not yet supported' module export syntax
    • life is better when you don't transpile, the only excuse would be if we were using typescript and our consumers needed good typing info
  4. Don't set winston as a peer dependency. npm 6 doesn't have any kind of support for peer dependencies (yet) and if you install the dependency for testing then it adds it to the package. Not sure about this one....

TODO: test it with an actual, you know, aws firehose.

meh, looks like it works to me. here's what was in my s3 bucket:

{"timestamp":"2018-11-16T00:53:10.545Z","level":"info","message":"test message","meta":{"rich":"meta"}}
{"timestamp":"2018-11-16T00:57:39.522Z","level":"info","message":"test message","meta":{"rich":"meta"}}
{"timestamp":"2018-11-16T00:57:43.079Z","level":"info","message":"test message","meta":{"rich":"meta"}}

👍 :tractor:

Fixes: #14

pkallos commented 5 years ago

this is awesome @acinader thanks.

couple of questions:

acinader commented 5 years ago

Hi @pkallos

  1. I did test! you can see the test output in this PR's description. It looks good to me, but I don't have before and after that I could compare. i.e. I don't have records from the current version.

  2. I didn't test it as a module cause I don't have an active project that I am using this in! If someone does they could do a good before after test by using this branch in their package.json. Having said that, I don't think that it'll improve anything with my eyes, cause I still won't have before and after.

  3. We can choose the squash the commits option when we press the merge button.

pkallos commented 5 years ago

awesome looks great, will cut another version