FINERACT-822 Enable Warnings CompareToZero#951
Conversation
|
@vorburger, @awasum, @xurror LGTY? |
percyashu
changed the title
percyashu
force-pushed
the
FINERACT-822
branch
3 times, most recently
from
80cf392 to
ab7b4e7
Compare
xurror
left a comment
xurror
left a comment
There was a problem hiding this comment.
I'm seeing you are enabling many checks at once. I would propose you instead raise small PRs for each of them. It will become a real PITA to review this if to files and changes done here. Is that OK for you?
|
OK will revert to the first commit |
percyashu
changed the title
There was a problem hiding this comment.
Just curious of this change, @percyashu can you please tell why they mean the same thing?
There was a problem hiding this comment.
Always comparing to 0 is the safest use of the return value when using compareTo since the magnitude of the value return is not certain but the sign is. See https://errorprone.info/bugpattern/CompareToZero
There was a problem hiding this comment.
This is actually "fun"... I also had to read https://errorprone.info/bugpattern/CompareToZero - and now I understand it. What's really interesting and not just fun is that in N places of this PR you're just doing something to make EP happy, but in exactly 4 occurrences, you've actually just fixed real bugs - see FINERACT-1014; thank You, Error Prone!
|
/rebase |
github-actions
bot
force-pushed
the
FINERACT-822
branch
from
ffa513f to
0c85a5d
Compare
|
The build failure here was "just" due to the (very interesting!) finding of @ptuomola in FINERACT-885. FINERACT-1014 explains how this PR isn't only "nice", but actually fixes real bugs in API validation . |
| if (this.status != null) { | ||
| if ((Double) this.status.get("id") >= 0) { | ||
| hash += (Double) this.status.get("id"); | ||
| hash = (int) (hash + (Double) this.status.get("id")); |
4 participants
FINERACT-822