Polish unchecked warning suppression in KafkaEvent#4183
Polish unchecked warning suppression in KafkaEvent#4183artembilan merged 1 commit intospring-projects:mainfrom
Conversation
b0edb2c to
ee09f9c
Compare
artembilan
left a comment
There was a problem hiding this comment.
Thank you!
We can accept your change since you have already dedicated some effort to do this, however my concern is that this is not necessary since we have those only two times in the class. As far as I remember SonarQube starts complaining when we have four repetitions of the same string constant.
At the same time I'm not sure why would one complain about Java's built-in unchecked word. It could be expressed as Java's built-in constant itself, or a respective optimization could be done by compiler. Don't know what is going on in Java internals, but I don't see a reason to worry about @SuppressWarnings values optimizations.
At the same time I'd suggest to look into this article for better commit message: https://cbea.ms/git-commit/.
You said already enough in the PR descriptions, but that one has nothing to do with the commit that going to be merged. There is just not PR in the commit history.
At the same time GH is smart enough to turn the first commit message into a PR description. This way we got a gain: we write what was changed and why only once in the commit message. Then we won't need to add PR description and commit history will be happy.
Thank you!
ee09f9c to
a0c63f4
Compare
|
Thanks for the feedback and for pointing me to the git commit message guide! I’ve amended the commit message to better describe what changed and why, Thank you for taking the time to think through this from my perspective |
Replace repeated "unchecked" string literals in @SuppressWarnings with a private static UNCHECKED constant. This is a cosmetic refactoring and does not change KafkaEvent behavior. Signed-off-by: LeeJaeHyeok97 <hazardous10@naver.com>
a0c63f4 to
dac197b
Compare
|
Will be merged when build is green. |
Summary
This PR refines the
@SuppressWarningsusage inKafkaEventby introducing a constant for the"unchecked"warning name:UNCHECKEDinKafkaEvent.@SuppressWarnings(UNCHECKED)for bothgetContainer(...)andgetSource(...).Motivation
This is a small polish to avoid repeating the same warning name string literal and centralize it as a constant.
The behavior of
KafkaEventremains unchanged.Testing