Skip to content

Conversation

@shan305
Copy link

@shan305 shan305 commented Dec 31, 2025

I built the project on Java 17 and noticed the README still referenced
legacy CMS GC flags, which are no longer supported on modern Java
versions.

This PR updates the README with modern JVM guidance and clarifies
Java 17 compatibility.

@coderabbitai
Copy link

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Summary by CodeRabbit

  • Documentation
    • Updated JVM tuning guidance: added a consolidated "JVM notes" section, clarified that examples run on modern JDKs (Java 17+), removed legacy CMS-specific tuning, and now recommends contemporary GC choices (G1 by default; ZGC or Shenandoah for low-latency on supported JDKs).

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

README.md updated to remove CMS-specific JVM tuning lines and replace them with a new "JVM notes" section that recommends modern GC collectors (G1 default, ZGC/Shenandoah as appropriate) and notes Java 17 compatibility for running the examples.

Changes

Cohort / File(s) Summary
Documentation Update
README.md
Replaced CMS-specific JVM tuning flags and SurvivorRatio details with a generalized "JVM notes" section recommending modern GC choices (G1, ZGC, Shenandoah) and added Java 17 compatibility guidance.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating README documentation with modern JVM guidance for Java, which aligns with the changeset that replaces legacy CMS GC flags with modern GC collector recommendations.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation (legacy CMS flags no longer supported in Java 17) and the fix (updates README with modern JVM guidance and Java 17 compatibility).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fa8004 and 9c9c462.

📒 Files selected for processing (1)
  • README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

66-66: Heading style
Expected: setext; Actual: atx

(MD003, heading-style)

🔇 Additional comments (1)
README.md (1)

66-71: Content approved; formatting concern from previous review has been addressed.

The JVM notes section provides accurate and helpful guidance aligned with the PR objectives:

  • Java 17 compatibility is correctly documented
  • Modern GC recommendations (G1 default, ZGC/Shenandoah alternatives) are factually sound
  • CMS removal warning is accurate and useful for developers upgrading from older Java versions

The blank line after the heading (line 67) resolves the prior MD022 formatting concern. The MD003 linting hint (heading style) is a non-issue since the entire README consistently uses ATX-style headings throughout, making this choice appropriate and intentional.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d28a5f1 and 4fa8004.

📒 Files selected for processing (1)
  • README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md

66-66: Heading style
Expected: setext; Actual: atx

(MD003, heading-style)


66-66: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

@shan305
Copy link
Author

shan305 commented Dec 31, 2025

Thanks for the review! I’ve addressed the markdown spacing feedback by adding a blank line after the section heading. Happy to adjust further if needed.

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.

1 participant