Skip to content
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: PartitionInfo toString corrected #2306

Closed
wants to merge 3 commits into from
Closed

Conversation

kamalcph
Copy link
Collaborator

@kamalcph kamalcph commented Jan 4, 2017

  • Removed the extra ',' character while printing the replicas / in-sync replicas identifier

- Removed the extra ',' character while printing the replicas / in-sync replicas identifier
@asfbot
Copy link

asfbot commented Jan 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/473/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link

asfbot commented Jan 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/471/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link

asfbot commented Jan 4, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/472/
Test PASSed (JDK 8 and Scala 2.12).

@ijuma
Copy link
Contributor

ijuma commented Jan 4, 2017

Thanks for the PR. Can you please add a unit test? Since the original bug eluded us, it indicates that a simple test that validates the output would be useful.

@@ -80,13 +80,10 @@ public String toString() {
/* Extract the node ids from each item in the array and format for display */
private String fmtNodeIds(Node[] nodes) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please rename this to formatNodeIds. The name is unnecessarily cryptic (unrelated to this PR, I know).

@asfbot
Copy link

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/510/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/509/
Test FAILed (JDK 7 and Scala 2.10).

- Removed the extra ',' character while printing the replicas / in-sync replicas identifier
- PartitionInfoTest added for `toString` method.
- Renamed the method 'fmtNodeIds' to 'formatNodeIds'
@kamalcph
Copy link
Collaborator Author

kamalcph commented Jan 5, 2017

@ijuma

Updated the method name and unit test added.

@asfbot
Copy link

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/513/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/514/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/512/
Test FAILed (JDK 7 and Scala 2.10).

@ijuma
Copy link
Contributor

ijuma commented Jan 5, 2017

LGTM, thanks. Merged to trunk.

@asfgit asfgit closed this in d357264 Jan 5, 2017
soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
Removed the extra ',' character while printing the replicas / in-sync replicas
array.

Author: Kamal <kamal@nmsworks.co.in>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes apache#2306 from Kamal15/trunk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants