Some fixes, refactoring and updates

ancashoria / graphql-kafka-subscriptions

Apollo graphql subscriptions over Kafka protocol

MIT License

187 stars 55 forks source link

Some fixes, refactoring and updates #24

Closed fmannhardt closed 4 years ago

fmannhardt commented 4 years ago

I am using this package in a project and needed some changes. I saw that you are not investing time anymore in this package, so I created a fork with some updates and some refactoring as:

Providing the publish method matching the actual PubSubEngine interface
Simplifying the event channel code using event emitter
Support for message headers and custom message key function
Integration tests with Docker
More error handling
Other small changes

Not sure if you want to have those changes here since some of them are more a refactoring or if I should simply maintain a fork? I am not really a TypeScript/JavaScript developer, but have an interest in getting this maintained. So happy to help.

Published a version here in the meantime as I cannot wait: https://www.npmjs.com/package/@fmannhardt/graphql-kafka-subscriptions

ancashoria commented 4 years ago

Hi @fmannhardt, you are right I'm not investing any more time in this project since I'm not using Kafka at the moment.

If you have time to invest in this, I'll happily transfer the repository to you and also give you the npm publishing rights.

How does that sound?

fmannhardt commented 4 years ago

That is also fine. It will take a while for me to find the time to write proper documentation, but I can definitely invest a bit of time to get this to the next major version.

rodrigopinto commented 4 years ago

Hey @ancashoria and @fmannhardt, I also have interest in helping this codebase for production use and I would be willing to invest time and help with maintenance, documentation and support to get it into a major version as well.

So, I would be happy to see this PR merged and happy to help with it as well.

ancashoria commented 4 years ago

Hey @fmannhardt and @rodrigopinto I have added you as collaborators on github. Is this enough for repository access? I'd like to also invite you as a maintainer or https://www.npmjs.com/package/graphql-kafka-subscriptions and I need your npm usernames for that.

rodrigopinto commented 4 years ago

@ancashoria thanks for the invite, my user on npm is https://www.npmjs.com/~rodrigoospinto

fmannhardt commented 4 years ago

@ancashoria thanks and sorry the the slow response. Here my npm user: https://www.npmjs.com/~fmannhardt

@rodrigopinto thanks and nice that you are joining. I will try to merge this in the next days. I think we should dump the version number to 1.0.0 for this one as there is at least one breaking change:

The publish method now follows the interface of gaphql-subscription and is public async publish(channel: string, payload: any): Promise<void> instead of public async publish(payload: any): Promise<void>

rodrigopinto commented 4 years ago

@fmannhardt I understand we follow a SemVer, but do you think it would be an issue to bump to 0.4.X? I mean, I personally don't know if the package can be considered "1.0.0", and that's how I experience few projects I contributed before dealing with versions previous to 1.0. If you say it is ready, let's do it, in case not, we bump intermediate releases until we are sure it is 1.0.0, but is your call. 😄

fmannhardt commented 4 years ago

I agree, probably 1.0.0 would give the wrong impression. Will try to find some time next week.

fmannhardt commented 4 years ago

Until then I merged this into a new dev branch.