refactor: add check for missing asm dependency#20
Conversation
WalkthroughAdded ASM library optional dependency detection and runtime validation. Introduced an IS_ASM_PRESENT static flag in ClassInstrumentationUtil to detect ASM availability at class load time, with a runtime check that throws IllegalStateException if absent. Updated README with ASM dependency documentation for Vaadin 14-23 support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
155-162: Specify the language for the fenced code block.The code block should specify
xmlas the language for proper syntax highlighting and to satisfy linting rules.This feature requires a dependency with ASM (which is not provided out-of-the-box in Vaadin 14-23): -``` +```xml <dependency> <groupId>org.ow2.asm</groupId> <artifactId>asm</artifactId> <version>9.8</version> </dependency></blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5086a28a848338c0fd13aa894ebbebfeceb1b381 and fb0c568162fd6e03aa4855c0371e574d596f0cdf. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `README.md` (1 hunks) * `src/main/java/com/flowingcode/vaadin/jsonmigration/ClassInstrumentationUtil.java` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (1)</summary> <details> <summary>📚 Learning: 2025-11-25T16:35:42.544Z</summary>Learnt from: javier-godoy
Repo: FlowingCode/JsonMigrationHelper PR: 11
File: src/main/java/com/flowingcode/vaadin/jsonmigration/JsonSerializer.java:281-301
Timestamp: 2025-11-25T16:35:42.544Z
Learning: In the JsonMigrationHelper project, code copied from the Vaadin codebase should be kept consistent with the original source unless there's a specific reason to deviate.**Applied to files:** - `README.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>README.md</summary> 156-156: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>src/main/java/com/flowingcode/vaadin/jsonmigration/ClassInstrumentationUtil.java (2)</summary><blockquote> `68-80`: **LGTM! Clean ASM presence detection.** The static initializer correctly uses `Class.forName` with `initialize=false` to detect ASM availability without triggering class initialization. This approach allows the class to load even when ASM is absent. --- `140-142`: **Good placement of the ASM check.** The check is correctly positioned after `needsInstrumentation` - this way, classes that don't require instrumentation will work fine even without ASM. The error message with exact Maven coordinates is helpful for developers. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Summary by CodeRabbit
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.