Skip to content

Consumer, Producer, TopicPartition classes are now sub-classable #63

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

Merged
merged 1 commit into from
Nov 14, 2016

Conversation

edenhill
Copy link
Contributor

@edenhill edenhill commented Nov 9, 2016

No description provided.

Copy link
Contributor

@ewencp ewencp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edenhill Just some issues with formatting tabs/spaces, but otherwise LGTM

@@ -55,7 +55,7 @@ static void Consumer_dealloc (Handle *self) {
if (self->rk)
rd_kafka_destroy(self->rk);

Py_TYPE(self)->tp_free((PyObject *)self);
Py_TYPE(self)->tp_free((PyObject *)self);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be getting mixed tabs & spaces...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file originally has tabs for indention, which sucks but I probably coded it with the wrong .emacs file in place.
New changes use space for indent instead.
We can do a forklift untabify at some point.

@edenhill
Copy link
Contributor Author

@roopahc and @criccomini I'd like you to try this out before I merge it.

@roopahc
Copy link
Contributor

roopahc commented Nov 14, 2016

@edenhill @ewencp I tested with AvroProducer class. It works good.. Thanks for this update.

@edenhill
Copy link
Contributor Author

@roopahc Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants