gnodet-bot commented on code in PR #13881:
URL: https://github.com/apache/maven/pull/13881#discussion_r4501514101
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java:
##########
@@ -266,8 +264,8 @@ void supportsFailurePreventsXmlFallback() throws Exception {
}
@Test
- void unnamedSupportsFailurePreventsXmlFallback() throws Exception {
- assertSupportsFailureStopsSelectionAndParsing(null, false);
+ void supportsFailurePreventsXmlFallback() throws Exception {
Review Comment:
🚨 **Compile error — duplicate method name.** The class already has a
`supportsFailurePreventsXmlFallback()` method (shown as the surrounding
function in the hunk header at old line 266). Renaming
`unnamedSupportsFailurePreventsXmlFallback` to the same name creates two
methods with identical signatures, which will not compile.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java:
##########
@@ -509,17 +509,20 @@ void unnamedParserWorksInNativeContainer() throws
Exception {
}
@Test
- void unnamedParserConflictIsReported() throws Exception {
- var parsers = new HashMap<String, SettingsParser>();
- parsers.put(null, new PropertiesSettingsParser());
- parsers.put("properties", new PropertiesSettingsParser());
+ void parserConflictIsReported() throws Exception {
Review Comment:
🔧 **Missed rename — sibling method `unnamedParserWorksInNativeContainer` not
renamed.** `unnamedParserConflictIsReported` was renamed to
`parserConflictIsReported` here, but its sibling
`unnamedParserWorksInNativeContainer` (a few lines earlier) was not updated.
The `unnamed` prefix convention is being removed with the introduction of
`id()` — rename it to `parserWorksInNativeContainer` for consistency.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultSettingsParserTest.java:
##########
@@ -276,24 +274,27 @@ void supportsFailurePreventsPreviouslyMatchedParser()
throws Exception {
}
@Test
- void unnamedSupportsFailurePreventsPreviouslyMatchedParser() throws
Exception {
- assertSupportsFailureStopsSelectionAndParsing(null, true);
+ void supportsFailurePreventsPreviouslyMatchedParser() throws Exception {
Review Comment:
🚨 **Compile error — duplicate method name.** The class already has a
`supportsFailurePreventsPreviouslyMatchedParser()` method (shown as the
surrounding function in the hunk header at old line 276). Renaming
`unnamedSupportsFailurePreventsPreviouslyMatchedParser` to the same name
creates two methods with identical signatures, which will not compile.
##########
its/core-it-suite/src/test/resources/mng-7836-alternative-pom-syntax/maven-hocon-extension/src/main/java/org/apache/maven/hocon/HoconModelReader.java:
##########
@@ -41,6 +41,12 @@
@Priority(1)
public class HoconModelReader implements ModelParser {
+
+ @Override
Review Comment:
🔧 **Style — extra blank line before `@Override`.** The existing blank line
after the class opening `{` (or previous member) plus the new blank added
before `@Override` creates a double-blank gap. Remove the extra leading blank —
the trailing blank after `}` is fine. The same pattern applies to
`DumbModelParser1`–`4` and `PropertiesSettingsParser` (mng-8655).
--
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]
