search

narcisoguillen / kafka-node-avro

ISC License
26 stars 13 forks source link

kafka-node should be a peerDependency #3

Closed crobinson42 closed 5 years ago

narcisoguillen commented 5 years ago

Hey thanks a lot for using kafka-node-avro, why is that ? wouldn't that cause an issue if an older or newer version of kafka-node is install as peer and does not work with the version that kafka-node-avro is test it with ?

crobinson42 commented 5 years ago

Well if you stick with with your major versioning in parallel with kafka-node, it technically shouldn't break anything.

crobinson42 commented 5 years ago

The problem with it being a dependency in this project is the manual requirement for always staying up to date with publishing a new release when kafka-node changes - what if you get hit by a bus today and kafka-node publishes a new release tomorrow... just saying :-)

narcisoguillen commented 5 years ago

Oh good point, I like the way you think. I'll migrate kafka-node dependency to a peer dependency and release as 1.0.2

crobinson42 commented 5 years ago

Well that’s a breaking change - also the latest version of Kafka-node is 4.1.3

So this project should release 4.0.0 with Kafka-node as a peerDep - and I would make it a statement on the README so everyone is aware.

And finally, probably make a graceful/descriptive error that throws on start-up if require('kafka-node’) fails to load, ie: it’s not installed

On Jun 28, 2019, at 3:22 PM, Narciso notifications@github.com wrote:

Oh good point, I like the way you think. I'll migrate kafka-node dependency to a peer dependency and release as 1.0.2

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/narcisoguillen/kafka-node-avro/issues/3?email_source=notifications&email_token=ABNSMSZYH77MIQNBZUEPOKTP42FLFA5CNFSM4H4AQ2N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3JNNQ#issuecomment-506894006, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNSMS7ITVHYYU6TKYTXHRTP42FLFANCNFSM4H4AQ2NQ.

narcisoguillen commented 5 years ago

Would you like to help me out making this package better with a contribution ? that would be awesome.

crobinson42 commented 5 years ago

Yeah! I’ll try and put a PR together when I have some time next week.

On Jun 28, 2019, at 3:30 PM, Narciso notifications@github.com wrote:

Would you like to help me out making this package better with a contribution ? that would be awesome.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/narcisoguillen/kafka-node-avro/issues/3?email_source=notifications&email_token=ABNSMSYFGM2GS7OUIQDCEVLP42GIVA5CNFSM4H4AQ2N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY3JYLI#issuecomment-506895405, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNSMS7LGVOYWMJBPFTSDZTP42GIVANCNFSM4H4AQ2NQ.

narcisoguillen commented 5 years ago

Hey thanks, here a PR I made, let me know what you think : https://github.com/narcisoguillen/kafka-node-avro/pull/4