Repository navigation
Fix the 117 PMD violations and gate pmd:check at verify (#114) - #120
Merged
Conversation
Substantive fixes: - Delete DecoderRunner, the unreferenced main() harness (27 violations, see #99) - TypeConverter.decode1sComplement: replace -(~result) by result + 1 with a comment; add TypeConverterTest for 1's and 2's complement decoding - GetFrusRunner: log FRU chunk read and decode failures instead of swallowing them, so a lost packet truncating a FRU is visible (see #102) - AbstractIpmiRunner.close(): log the closeSession failure at debug - ConnectionManager, MessageQueue, UdpMessenger: restore the interrupt flag instead of swallowing InterruptedException (cancellation rework stays in #79) - SerialOverLan.waitForData: sleep 1 ms per iteration instead of busy-waiting - ReadFruData: drop the dead else-if (false) branch, keep the SPD TODO (#107) - UdpMessenger/UdpNotifier: drop super.run() from run(); bind the wildcard address with null instead of the hard-coded "0.0.0.0" - IpmiAsyncConnector.closeSession: drop the useless return Mechanical fixes: unnecessary self-qualifications in TypeConverter, the FRU records and SolAckState; useless parentheses in GetChassisStatusResponseData, CipherSuite, TypeConverter, GetSensorReading and FullSensorRecord. Build: run pmd:check at verify with pmd.xml (cpd-check awaits #115) and pin the reporting plugin to the same 3.28.0 so the site report matches the gate. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a1fde36e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
wait(1) clears the interrupt flag when it throws, so restoring it inside the loop made every following wait(1) throw immediately: an interrupted request hot-spun while all 60 sessionless tags were reserved. Remember the interrupt and restore it when the method returns; add a saturated-pool test. Co-Authored-By: Claude Fable 5.1 <[email protected]>
bertysentry
deleted the
114-pmd-fix-the-117-violations-reported-with-the-project-ruleset-empty-catch-blocks-threadrun-always-true-if-unary-operators-qualified-names-parentheses
branch
October 7, 2026 19:29
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #114.
What changed
Substantive (16 violations)
DecoderRunner, themain()harness nothing referenced (27 violations on its own; part of Dead code and leftovers in core: UdpNotifier, SessionUpkeep, DecoderRunner main() in src/main, unused queue methods, dead UdpMessenger bufferSize #99).TypeConverter.decode1sComplement: the flagged-(~result)was correct but obscure; it is nowresult + 1with a comment. NewTypeConverterTestpins 1's and 2's complement decoding.GetFrusRunner: FRU chunk read and decode failures are now logged at warn instead of swallowed, so a lost packet truncating a FRU is visible (see getFrusAndSensorsAsStringResult opens two sessions and walks the SDR repository twice; FRU reads are slow (16-byte chunks) #102).AbstractIpmiRunner.close()logs thecloseSessionfailure at debug.ConnectionManager,MessageQueue,UdpMessenger: the swallowedInterruptedExceptions now restore the interrupt flag. The wall-clock/cancellation rework stays in Timeouts and cancellation: waitForResponse counts sleeps instead of wall-clock, swallows InterruptedException, and non-daemon threads keep the JVM alive #79.SerialOverLan.waitForData: sleeps 1 ms per iteration instead of busy-waiting, and exits on interrupt.ReadFruData: deadelse if (false)branch removed; the SPD TODO now points at Decode SPD records from DIMM FRUs (SpdInfo is a stub) #107.UdpMessenger/UdpNotifier:super.run()removed fromrun()(it was a no-op), the hard-coded0.0.0.0replaced by a null bind address (wildcard). Uselessreturndropped inIpmiAsyncConnector.Mechanical (101 violations)
TypeConverter, the FRU records andSolAckState; useless parentheses removed inGetChassisStatusResponseData,CipherSuite,TypeConverter,GetSensorReading,FullSensorRecord.Build
pmd:checknow runs atverifywithpmd.xml, as in winrm-java.cpd-checkis left out until CPD: remove the 421 duplicated lines shared by FullSensorRecord, CompactSensorRecord and EventOnlyRecord #115 lands.maven-pmd-pluginis pinned to the same 3.28.0 so the site report shows what the gate checked (the parent's 3.26.0 / PMD 7.7.0 also cannot read JDK 25 class files).Verification
mvn verify: BUILD SUCCESS, 0 Checkstyle violations, 0 PMD violations, 25 tests pass.mvn verify sitestill fails locally on JDK 25, but only in the SpotBugs report, exactly as onmain(documented in AGENTS.md). CI runs JDK 17.Reviewer notes
UdpMessenger(int)keeps itsthrows UnknownHostExceptionclause for source compatibility even though nothing throws it any more.🤖 Generated with Claude Code