Skip to content

KAFKA-18699: Cleanup Metadata Module #20346

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

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

sjhajharia
Copy link
Contributor

@sjhajharia sjhajharia commented Aug 12, 2025

https://issues.apache.org/jira/browse/KAFKA-18699

This PR aims at cleaning up the metadata module further by getting rid
of some extra code which can be replaced by record

@github-actions github-actions bot added triage PRs from the community kraft labels Aug 12, 2025
Copy link
Collaborator

@mingyen066 mingyen066 left a comment

Choose a reason for hiding this comment

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

The cleanup for migrating suitable classes to records has a corresponding JIRA ticket: KAFKA-18696.

For metadata cleanup, you can take my ticket: KAFKA-18699.

return other.current == current && other.next == next;
}

record BrokerControlStates(BrokerControlState current, BrokerControlState next) {
@Override
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not remove toString as well? As far as I know, the only difference is that the parentheses become square brackets.

@github-actions github-actions bot removed the triage PRs from the community label Aug 14, 2025
@sjhajharia sjhajharia changed the title MINOR: Cleanup Metadata Module KAFKA-18699: Cleanup Metadata Module Aug 14, 2025
@sjhajharia
Copy link
Contributor Author

Thanks @mingyen066
I have assigned the JIRA to myself and as per your suggestion got rid of some of the toString() methods as well. I did not touch the ones which use Stringify in them.
Re-requesting a review.

@sjhajharia sjhajharia requested a review from mingyen066 August 14, 2025 05:46
Copy link
Collaborator

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

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

Thanks @sjhajharia for this patch, left some comments

@sjhajharia
Copy link
Contributor Author

Thanks @m1a2st for the review. I have addressed the comments. Pls re-review. Thanks!

@sjhajharia sjhajharia requested a review from m1a2st August 15, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants