Skip to content

Conversation

@peter-lawrey
Copy link
Member

@peter-lawrey peter-lawrey commented Oct 27, 2025

This change introduces a Chronicle-standard code-review profile and aligns the module with our house quality rules. It wires Checkstyle, PMD, SpotBugs(+FindSecBugs), and JaCoCo using pinned versions, codifies coverage thresholds, and adds narrowly-scoped suppressions with justifications. While here, it makes a few low-risk refactors and documentation tweaks to improve determinism, readability, and consistency.

Why

  • Establish a repeatable, team-wide quality baseline that matches sibling projects.
  • Prevent regressions in concurrency, error handling, and API hygiene.
  • Make findings actionable by shipping local suppressions with rationale.
  • Ensure docs render consistently with numbered sections.

What changed

Build & Quality

  • Add pinned plugin and ruleset properties in pom.xml:

    • checkstyle, puppycrawl, spotbugs, findsecbugs, maven-pmd-plugin, jacoco-maven-plugin
    • Chronicle rules: chronicle-quality-rules.version=1.23ea6
  • Centralise JaCoCo version via ${jacoco-maven-plugin.version} and enforce minimum coverage gates:

    • Line coverage: >= 0.80
    • Branch coverage: >= 0.70
  • New local suppression files:

    • src/main/config/pmd-exclude.properties
    • src/main/config/spotbugs-exclude.xml
    • Each entry has an ID and comment (e.g. JLBH-OPS-20x) explaining the trade-off.

Production code

  • JLBH.java

    • Replace volatile long noResultsReturned with AtomicLong sampleCount for clear atomicity and simpler timeout checks.
    • Reset logic and warmup progress printing updated to use sampleCount.
    • Simplify modulus growth loop (while), minor Javadoc and formatting tidy-ups.
  • TeamCityHelper.java

    • Remove redundant lambda parentheses; no functional change.
  • JLBHResultSerializer.java

    • Write CSV via Files.newBufferedWriter(..., UTF_8) for explicit encoding and simpler IO handling.

Documentation

  • Enable :sectnums: and normalise section titles across:

    • architecture.adoc
    • benchmark-lifecycle.adoc
    • jlbh-cookbook.adoc
    • project-requirements.adoc
    • results-interpretation-guide.adoc

How to run the new checks

  • Local: mvn -Pcode-review -q verify
  • CI: profile can be toggled in the workflow to gate on Checkstyle/PMD/SpotBugs/JaCoCo plus SonarCloud (sonar.organization=openhft, sonar.host.url=https://sonarcloud.io already present).

Expected outcomes

  • Deterministic, actionable findings from style/static analysis.
  • Reproducible coverage thresholds enforced at build time.
  • No API changes; runtime behaviour unchanged except safer, clearer sample counting.
  • TeamCity stats output remains identical.

Notable suppressions (justified)

  • PREDICTABLE_RANDOM in synthetic distributors (reproducibility preferred over crypto strength).
  • EI_EXPOSE_REP* in performance-critical paths where copying would inflate allocations.
  • PATH_TRAVERSAL_IN in CLI-driven serializer (paths supplied by operator scripts).
  • See XML/properties for IDs and comments linking to follow-ups (JLBH-OPS-20x).

Risk & rollback

  • Low risk: refactors are mechanical with unit-level surface area. If needed, revert via this commit to restore previous sample counter and IO path.

Follow-ups

  • Address remaining SpotBugs warnings tracked under JLBH-OPS-260 (soft-fail paths).
  • Consider promoting selected suppressions into the shared Chronicle ruleset if reused across modules.

@peter-lawrey peter-lawrey changed the title Adv/code review Introduce shared code-review profile and tighten quality gates for JLBH Oct 27, 2025
@sonarqubecloud
Copy link

@peter-lawrey peter-lawrey marked this pull request as draft November 3, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants