search

nodefluent / node-sinek

:tophat: Most advanced high level Node.js Kafka client
MIT License
290 stars 52 forks source link

Update to remove old kafka modules #154

Closed rob3000 closed 4 years ago

rob3000 commented 4 years ago

Removing old node modules and use kafkaJS as primary source and removed some old config.

Suggestions/comments welcome 😄

Next i'd like to update some of the documentation thoughts on using something like: https://docusaurus.io/

krystianity commented 4 years ago

Looking good @rob3000 I am on the road currently, going to have a look at this on the weekend.

rob3000 commented 4 years ago

@krystianity updated based on the comments you made. I've also converted to typescript. Currently admin client is failing but feel free to review :)

rob3000 commented 4 years ago

@krystianity This is now ready for review! 🥳

krystianity commented 4 years ago

Awesome, will review tonight - you are really going full speed here 👍 ❤️

krystianity commented 4 years ago

looking good so far, however the amount of changes is quite large so its quite difficult to approve this with 100% confidence, given the major changes we are doing anyway, we will probably have to run some in the fields tests for lag status (health and analytics) - testing this with kafka-streams will probably also be a good validation of the changes.

rob3000 commented 4 years ago

agreed, there are lots of changes. i guess if we merge to master we can test kafka streams from the sinek's master branch which would give us better confidence?

krystianity commented 4 years ago

@rob3000 didnt release you were waiting for my approval to merge this, lgtm ;)