Skip to content

Conversation

@jeffwidman
Copy link
Contributor

@jeffwidman jeffwidman commented Oct 11, 2019

Currently these are TODO's, but hopefully @dpkp can provide backstory on whether to remove these or leave them in...


This change is Reviewable

@jeffwidman jeffwidman force-pushed the remove-some-unused-code branch from 97b0e05 to 1e1b13d Compare October 11, 2019 19:35
with self._lock:
return len(self._waiters)

# TODO should this be removed or finished?
Copy link
Owner

Choose a reason for hiding this comment

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

This is fine to remove. Memory allocation is not a bottleneck as far as I can tell.

return False
return True

# TODO should this be removed or finished?
Copy link
Owner

Choose a reason for hiding this comment

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

Yep -- remove

kafka/util.py Outdated
pass


# TODO what is the purpose of this? It has been unused since it was originally added
Copy link
Owner

Choose a reason for hiding this comment

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

I was thinking about using this to help prevent circular references when calling instance methods with atexit . Currently we only do that in the producer, and we use weakproxy directly. If we ever needed to do a similar cleanup for other classes then this method might be helpful. But I don't mind if you delete it.

Repository owner deleted a comment from dpkp Feb 5, 2020
@jeffwidman jeffwidman force-pushed the remove-some-unused-code branch from 1e1b13d to 1dace1d Compare February 5, 2020 23:54
@jeffwidman jeffwidman force-pushed the remove-some-unused-code branch from 1dace1d to ca2d763 Compare February 5, 2020 23:56
@jeffwidman jeffwidman merged commit 6babefa into master Feb 5, 2020
@jeffwidman jeffwidman deleted the remove-some-unused-code branch February 5, 2020 23:57
jeffwidman added a commit that referenced this pull request Feb 6, 2020
Forgot to remove this in #1925
/ ca2d763
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