[GitHub] [incubator-eventmesh] Markliniubility commented on pull request #1014: [ISSUE #1018] Kafka Connector: Initial Implementation of Consumer

[GitBox <gi...@apache.org>]

Markliniubility commented on PR #1014:
URL: https://github.com/apache/incubator-eventmesh/pull/1014#issuecomment-1193945045

   > Hi, thanks for your initial implementation. Here are some suggestions.
   > 
   > This pr just implements consumer without enough tests. I recommend you implement producer first and verify it through local kafka server. If you could write some UT with kafka testcontainer, it will be much better. After that, we could have a base to implement consumer correctly.
   > 
   > What's more, please replace rocketmq with kafka in your code and comment.
   
   Thank you for reviewing! I agree with you on implementing the producer first (which I should have done first) and providing unit tests, and I am working on implementing the producer. However, the branch was only going to merge into a development branch, kafka-connector, instead of the master branch. I was going to add the unit tests and producer in separate PRs and make sure everything works and is fully tested before rolling out to the master branch. I prefer splitting the project into [small PR](https://google.github.io/eng-practices/review/developer/small-cls.html)s since it can be more easily reviewed and understood. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@eventmesh.apache.org
For additional commands, e-mail: dev-help@eventmesh.apache.org