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

Small refactoring to improve readability and reduce method complexity #2015

Closed
wants to merge 4 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@picadoh

picadoh commented Oct 12, 2016

Small method extraction to reduce complexity and improve readability of the assign method.

Private methods were created for:

  • Get the assignment suppliers based on the subscriptions (decoding of subscriptions info is also done at this point, where it is used)
  • Calculate the number of partitions for internal topic
  • Add tasks to state change log topic subscribers

These methods are being called inside the assign method.

@picadoh

This comment has been minimized.

Show comment
Hide comment
@picadoh

picadoh Oct 14, 2016

The failures are somewhat non-deterministic and seem to be unrelated with the change, can someone please double check?

Btw, in general all builds running against ubuntu-5 jenkins instance are failing due to timeout. There's a open infra jira issue to sort this out.

picadoh commented Oct 14, 2016

The failures are somewhat non-deterministic and seem to be unrelated with the change, can someone please double check?

Btw, in general all builds running against ubuntu-5 jenkins instance are failing due to timeout. There's a open infra jira issue to sort this out.

@picadoh picadoh closed this Oct 24, 2016

@guozhangwang

This comment has been minimized.

Show comment
Hide comment
@guozhangwang

guozhangwang Oct 24, 2016

Contributor

Hello @picadoh thanks for the patch. Could you take a look at #2012 and see if it covers your concerns?

Contributor

guozhangwang commented Oct 24, 2016

Hello @picadoh thanks for the patch. Could you take a look at #2012 and see if it covers your concerns?

@picadoh

This comment has been minimized.

Show comment
Hide comment
@picadoh

picadoh Oct 24, 2016

yep :) thanks @guozhangwang, great stuff.

picadoh commented Oct 24, 2016

yep :) thanks @guozhangwang, great stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment