Copilot commented on code in PR #274: URL: https://github.com/apache/datasketches-memory/pull/274#discussion_r2849933726
########## src/test/resources/testng.xml: ########## @@ -0,0 +1,28 @@ +<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd";> + Review Comment: The TestNG suite references an external DTD URL in the DOCTYPE, which can introduce a network dependency (offline/air‑gapped builds may fail or stall) and can also be blocked by XML security hardening. Consider removing the DOCTYPE entirely or pointing to a bundled/local DTD so test execution doesn’t require outbound access. ```suggestion ``` ########## pom.xml: ########## @@ -271,14 +279,18 @@ under the License. <artifactId>maven-surefire-plugin</artifactId> <version>${maven-surefire-failsafe-plugins.version}</version> <configuration> - <argLine>${jvm-arguments}</argLine> + <argLine>@{argLine} @{jvm.args}</argLine> Review Comment: Surefire `@{...}` late-property interpolation is primarily intended for `argLine` so JaCoCo can inject its agent; using `@{jvm.args}` (with dots in the property name) risks not being interpolated and being passed literally to the JVM. Safer approach is to keep `@{argLine}` but reference `${jvm.args}` normally so Maven resolves it reliably. ```suggestion <argLine>@{argLine} ${jvm.args}</argLine> ``` -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
