-
-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: kafka adapter #342
feat: kafka adapter #342
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great first approach, @Ruchip16 👏
Left some comments that I think can have a positive impact. Please, also remove all the package-lock.json changes from this PR. They shouldn't be part of this pull request 🙏
Keep up the great work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I think your experimentation is going great. You have a big mess in the adapter file, so I recommend you clean it out and start slowly adding things that you are sure should be there. No commented code, no copy-pasted code, clean it all and start over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're close to having a subscriber ready! Keep pushing! 💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, for some reason this suggestion wasn't added before 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, made a full review. It's looking good. Just apply these changes and we're done :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more round. Let's go! It's close!
52c622e
to
75f6d4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LET'S GOOOO!
Pull Request Test Coverage Report for Build 4007665241Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOW YES! LET'S GOOOO 🚀
/rtm |
🎉 This PR is included in version 0.17.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Congratulations on finishing your mentorship program, Ruchi! 👏 |
Thank youuu ✨ |
Description
Opening a draft PR for issue #256
npm run dev
in thecd examples/dummy
folder in order to check whether our Kafka broker connects to the glee or not in that exampleRelated issue(s)