-
Notifications
You must be signed in to change notification settings - Fork 14.8k
MINOR: code cleanup in InternalTopicManager #21165
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
Conversation
| topicsStillToValidate.addAll(topicConfigsStillToValidate); | ||
|
|
||
| maybeThrowTimeout(new TimeoutContext( | ||
| new HashSet<String>() {{ |
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.
interesting syntax. Its like an array init, within the declaration, right?
Does is make an performance difference, or is it just error prone, thus some kind of community convent to avoid?
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.
Its like an array init, within the declaration, right?
Yes. Similar.
Does is make an performance difference, or is it just error prone, thus some kind of community convent to avoid?
It's not considered best practice. Overlooked during code-review. From Copilot:
Using double-brace initialization creates an anonymous inner class and can lead to memory leaks. Consider using a standard HashSet construction with addAll() calls or collect the items into a set using streams.
|
thanks for the hint, its even an error lvl warning. This should be done the right way to ensure its done overall and not just somewhere. Human interaction can not scale this issue accordingly. |
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
Signed-off-by: Vincent Potucek <[email protected]>
|
I agree that it would be great to fail the build for this. Would you be interested to look into it, and do a PR changing the gradle setup, and fix any potentially lingering code that uses double-brace initialization? |
|
already on it, thank you for initiative. |
lucasbru
left a comment
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
…afka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…afka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…afka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…afka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…afka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…apache.kafka.openrewrite.SanityCheck` apache#21165 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…pache#21161 Signed-off-by: Vincent Potucek <[email protected]>
…#21161 Signed-off-by: Vincent Potucek <[email protected]>
…#21161 Signed-off-by: Vincent Potucek <[email protected]>
…ache#21161 apache#21168 Signed-off-by: Vincent Potucek <[email protected]>
…ache#21161 apache#21168 Signed-off-by: Vincent Potucek <[email protected]>
Reviewers: Vincent Potuček (@Pankraz76), Lucas Brutschy
[email protected]