HADOOP-19731. Fix SpotBugs warnings introduced after SpotBugs version upgrade. #8053
+389
−41
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
36547f5 to
c46c6af
Compare
Contributor
|
@zhtttylz Thank you for following up on this issue. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
This was referenced Nov 4, 2025
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
366bf41 to
7b27c79
Compare
Contributor
|
@steveloughran @szetszwo We propose adding a global configuration to temporarily suppress the warnings generated by the new rules introduced in SpotBugs 4.9.7. Would this approach be acceptable to you? |
Contributor
@slfan1989, @zhtttylz , it sounds good. Thanks a lot for fixing the warnings! |
slfan1989
approved these changes
Nov 21, 2025
Contributor
|
suppressing the new reports would be mostly good (they are overreactions/false alarms), we may lose some real issues on the way. |
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
7b27c79 to
582a4f1
Compare
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
582a4f1 to
97a3b06
Compare
Contributor
steveloughran
left a comment
steveloughran
left a comment
There was a problem hiding this comment.
a lot of the java changes are just removing trailing spaces. Is this needed? I knowe it's not ideal to have these, but it makes for a bigger patch and cherrypicking harder. I treat them like out-of-order imports: something I leave alone except when editing the specific lines.
spotify doesn't complain about these lines, does it?
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
97a3b06 to
cbb96dc
Compare
Contributor
Author
@steveloughran Thanks for the review — your point about keeping the patch small for easier cherry-picks makes sense. I’ve dropped the trailing-whitespace cleanup and introduced pattern-level filters for the new 4.9.7 rules so we reduce noise while still surfacing real issues. The With the current revision, the pre-commit SpotBugs summary (PR-8053/10 at https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8053/10/consoleText) reports SpotBugs patch summary from PR-8053/9 |
slfan1989
reviewed
Nov 29, 2025
Contributor
slfan1989
left a comment
slfan1989
left a comment
There was a problem hiding this comment.
I generally agree with this PR, but there are a few minor issues.
| /** | ||
| * The contract of OBS: only enabled if the test bucket is provided. | ||
| */ | ||
| // CI: trigger SpotBugs (no-op) |
| return config; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Should this change be avoided as well?
Contributor
Author
There was a problem hiding this comment.
Thank you very much for taking the time to review this. The CI result from PR-8053/10 has already validated the new SpotBugs configuration, so I will reset that temporary commit to ensure this PR no longer includes the comment-only or formatting-only changes.
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
cbb96dc to
6414293
Compare
|
💔 -1 overall
This message was automatically generated. |
slfan1989
reviewed
Nov 30, 2025
| @@ -0,0 +1,56 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <FindBugsFilter> | |||
Contributor
There was a problem hiding this comment.
We need to add the Apache License notice.
Contributor
Author
There was a problem hiding this comment.
Thanks for the feedback—I’ll make the updates right away!
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
the license failures and s3a failures should all be fixed now: do a merge with trunk and non-forced update and yetus should be happy with hadoop-aws. No idea about the yarn/sls failures |
slfan1989
approved these changes
Dec 7, 2025
Contributor
zhtttylz
force-pushed
the
HADOOP-19731
branch
from
d2b2818 to
a076fda
Compare
Contributor
Author
|
@steveloughran @slfan1989 Thanks for the reminder. I’ve rebased HADOOP-19731 onto the latest trunk (with the ASF license fix from #8098) and pushed the updated branch so Yetus can rerun. Really appreciate your help here. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@zhtttylz I’m going to merge this PR, as the JavaDoc error is unrelated to these changes. Many thanks to @steveloughran and @szetszwo for reviewing the code. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
HADOOP-19731. Fix SpotBugs warnings introduced after SpotBugs version upgrade.
How was this patch tested?
Ran
mvn -Dspotbugs.skip=false spotbugs:spotbugson affected modules and verified the build no longer fails on SpotBugs warnings. No functional code changes, config-only.For code changes: