ammachado commented on PR #1670: URL: https://github.com/apache/maven-mvnd/pull/1670#issuecomment-5105178864
Thanks for the thorough review! Addressed all 7 points in acbb1477: **Medium** 1. `supportsForkNode` version gap — now only applies the milestone guard when `minor == 0 && patch == 0`, so `3.1.0-M2`/`3.2.5-M1` etc. are correctly treated as GA. Added a regression test for this. 2. `clearDisplay()` on failure — reverted to unconditional `clearDisplay()` at `BUILD_FINISHED`; the guarded version wasn't intentional. **Low** 3. Mutable list getters — `getFlakyTests()`/`getFailedTests()`/`getErroredTests()` now return `Collections.unmodifiableList(...)`. 4. `BANNED_MARKER` fragile match — agreed there's no programmatic alternative; added a comment documenting it as a maintenance risk so it's not silently missed if Maven changes the message. 5. `Pattern.compile` per call — hoisted to a `static final Pattern VERSION_PATTERN`. 6. Duplicated `isEnabled` logic — consolidated into `MvndTestProgressLifecycleParticipant.isTestProgressEnabled()`, called from both `Server.handle()` and the lifecycle participant. 7. Magic indices in the `int[]` snapshot — replaced with named `IDX_*` constants (records aren't available at this module's Java 8 target). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
