Skip to content

Adopt the MetricsHub formatter profile and jawk's checkstyle.xml, gate the build on both - #119

Merged
bertysentry merged 9 commits into
mainfrom
feature/issue-117-118-formatter-checkstyle
Oct 7, 2026
Merged

bertysentry merged 9 commits into
mainfrom
feature/issue-117-118-formatter-checkstyle

Conversation

@bertysentry

@bertysentry bertysentry commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #117, fixes #118.

Commits (please merge, don't squash: .git-blame-ignore-revs references the reformat commit by hash)

  1. Adopt the MetricsHub formatter profile and jawk's checkstyle.xml: metricshub-eclipse-formatter.xml and checkstyle.xml copied from jawk (same blob SHAs as jawk's main, except the allowInSwitchCase typo fix in checkstyle.xml, reported upstream as checkstyle.xml: trailing space in AvoidNestedBlocks property name allowInSwitchCase jawkio/jawk#612), formatter-maven-plugin 2.29.0 validate at the validate phase (main + test sources), maven-checkstyle-plugin 3.6.0 check with failOnViolation=true (same version and configLocation as the report inherited from oss-parent), .gitattributes from jawk (minus its jawk-specific src/site/resources/get rule), README "Code format" section, AGENTS.md updated.
  2. Reformat the whole tree with mvn formatter:format: pure formatting, 256 files.
  3. Fix the Checkstyle violations: the 72 errors left after the reformat (74 before: the formatter fixed 2 NewlineAtEndOfFile).
  4. .git-blame-ignore-revs listing commit 2 (git config blame.ignoreRevsFile .git-blame-ignore-revs, documented in README; GitHub uses it automatically).
  5. Review follow-up: protected getIpmiConfiguration() and getSik() accessors; trailing space removed from the allowInSwitchCase property name.

Checkstyle fixes

Check Fix
FinalClass (6) final on classes that only have private constructors
VisibilityModifier (10) protected fields of AbstractIpmiRunner, MessageHandler, IpmiLanMessage made private with protected accessors; ConfidentialityAlgorithm.sik and IntegrityAlgorithm.sik private with protected getSik() (IntegrityNone stores the key with the new protected setSik(), without MAC initialization)
HiddenField (22) parameters renamed (no behavior change)
ConstantName (9) logger → LOGGER, sessionlessTag → SESSIONLESS_TAG
NeedBraces (18), UpperEll (1), NewlineAtEndOfFile (2) mechanical
TypeName (3) suppressed with CHECKSTYLE.OFF/ON: renaming the public IntegrityHmacMd5_128, IntegrityHmacSha1_96, IntegrityHmacSha256_128 classes would break the API
AvoidStarImport (1) suppressed: ReadingTypeDescription is a lookup table over ~240 ReadingType constants

The 24 TodoComment warnings (TODO, tracked by #100, #106, #107, #108) don't fail the build.

API note: subclasses outside this library that accessed the former protected fields (AbstractIpmiRunner.connector/handle/nextRecId/ipmiConfiguration, MessageHandler.messageQueue/connection/lastReceivedSequenceNumber, IpmiLanMessage.networkFunction, ConfidentialityAlgorithm.sik, IntegrityAlgorithm.sik) must switch to the accessors (getIpmiConfiguration(), getConnector(), getHandle(), getNextRecId()/setNextRecId(), getMessageQueue(), getConnection(), getNetworkFunctionCode()/setNetworkFunctionCode(), getSik()/setSik(), getLastReceivedSequenceNumber()/setLastReceivedSequenceNumber()). These are protocol internals, not the IpmiClient API used by MetricsHub.

Verification (JDK 17, as in CI)

  • mvn clean verify site: BUILD SUCCESS, 23 tests pass, 0 Checkstyle violations, Javadoc jar (doclint all,-missing) has no warnings.
  • Gates work: a file I edited without re-running the formatter failed formatter:validate, and a leftover redundant final failed checkstyle:check.
  • Reports unchanged by the reformat: PMD 772, CPD 70, SpotBugs 1 on both main and this branch (same site configuration).
  • No real BMC involved; these changes rename and encapsulate but don't change protocol bytes.

🤖 Generated with Claude Code

bertysentry and others added 4 commits October 7, 2026 13:29
#118)

* metricshub-eclipse-formatter.xml and checkstyle.xml copied byte for byte from jawk
* formatter-maven-plugin 2.29.0 validates the main and test sources at the validate phase
* maven-checkstyle-plugin 3.6.0 check bound with failOnViolation=true (same version and
  configLocation as the report inherited from oss-parent)
* .gitattributes from jawk, so Java/XML/Markdown files are normalized to LF in the index
* README.md and AGENTS.md document mvn formatter:format and CHECKSTYLE.OFF/ON

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Pure formatting, no other change: produced by running mvn formatter:format with the
MetricsHub Eclipse formatter profile. Listed in .git-blame-ignore-revs.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
* FinalClass: IpmiClient, IpmiResultConverter, Utils, DeviceDescription,
  ReadingTypeDescription and MessageComposer only have private constructors
* VisibilityModifier: the protected fields of AbstractIpmiRunner, MessageHandler
  and IpmiLanMessage are now private with protected accessors; the never-read
  ConfidentialityAlgorithm.sik is removed and IntegrityAlgorithm.sik is private
* HiddenField: parameters renamed where they shadowed a field (no behavior change)
* ConstantName: static final loggers renamed LOGGER, sessionlessTag SESSIONLESS_TAG
* NeedBraces, UpperEll, NewlineAtEndOfFile: mechanical fixes
* Suppressed with CHECKSTYLE.OFF/ON: TypeName on IntegrityHmacMd5_128,
  IntegrityHmacSha1_96 and IntegrityHmacSha256_128 (public class names mirroring
  the IPMI algorithm names), AvoidStarImport on the ReadingType lookup table

The remaining 24 TodoComment warnings do not fail the build.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T13:10:59.141328Z 8bde132 New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2015b24b8

ℹ️ 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".

Comment thread checkstyle.xml Outdated
)

Review feedback: subclasses outside the library lost access to the former
protected fields. Add protected getIpmiConfiguration() to AbstractIpmiRunner and
getSik() to ConfidentialityAlgorithm and IntegrityAlgorithm (the
ConfidentialityAlgorithm.sik field is kept, now private).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Same typo as in jawk's checkstyle.xml, reported upstream as jawkio/jawk#612.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59f1a46d6f

ℹ️ 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".

Review feedback: getSik() promises the key passed to initialize(), which
IntegrityNone no longer stored. Add protected IntegrityAlgorithm.setSik() and
call it from IntegrityNone, which still skips the MAC initialization.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af0171c654

ℹ️ 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".

…#117)

Review feedback: subclasses lost access to the sliding-window state when the
field became private.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbe00f2705

ℹ️ 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".

Review feedback: the API change was described only in the PR. Add an
"Upgrading from 1.2.02" section to the site and point to it from README.md.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@bertysentry
bertysentry merged commit 4379014 into main Oct 7, 2026
4 checks passed
@bertysentry
bertysentry deleted the feature/issue-117-118-formatter-checkstyle branch October 7, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant