-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
MINOR: Add comment to onPartitionsLost override #14121
Conversation
Signed-off-by: Federico Valeri <[email protected]>
1c9f6c9
to
714d622
Compare
Signed-off-by: Federico Valeri <[email protected]>
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.
LGTM! Thanks for the improvement!
@fvaleri , one suggestion: I found you usually leave empty in the PR description, which is not good for reviewers. The PR description is the first place the reviewer will read before jumping into the code change. So if you can provide more info in the description, it'll make reviewer's job easier. I personally enjoy reviewing @divijvaidya 's PR a lot, because he explains everything clearly in the PR description. So you know what he is trying to achieve in the PR and which/why the method he chose...etc before jumping into the code. This PR is a good example. FYR!
Thanks @showuon, noted. |
@dajac are you good with these changes? |
Failed tests are unrelated. |
This adds comments to the ConsumerRebalanceListener overrides, in order to briefly explain why we are overriding these methods, when they are called, and what you can or can't do. Especially onPartitionsLost can create some confusion given the default implementation. Reviewers: Luke Chen <[email protected]>, David Jacot <[email protected]>
This adds comments to the ConsumerRebalanceListener overrides, in order to briefly explain why we are overriding these methods, when they are called, and what you can or can't do. Especially onPartitionsLost can create some confusion given the default implementation. Reviewers: Luke Chen <[email protected]>, David Jacot <[email protected]>
This adds comments to the ConsumerRebalanceListener overrides, in order to briefly explain why we are overriding these methods, when they are called, and what you can or can't do. Especially onPartitionsLost can create some confusion given the default implementation. Reviewers: Luke Chen <[email protected]>, David Jacot <[email protected]>
This adds comments to the ConsumerRebalanceListener overrides, in order to briefly explain why we are overriding these methods, when they are called, and what you can or can't do. Especially onPartitionsLost can create some confusion given the default implementation.