New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MNG-7354] Refactor MavenCli to ease extensibility #634
Conversation
Nice! |
maven-embedder/src/main/java/org/apache/maven/cli/logging/Slf4jLoggerManager.java
Outdated
Show resolved
Hide resolved
|
||
private static final String MVN_MAVEN_CONFIG = ".mvn/maven.config"; | ||
public static final String MVN_MAVEN_CONFIG = ".mvn/maven.config"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this property will likely go away with #602
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading #602, I'm not sure I agree with adding more intelligence in the launch scripts and removing it from the java code. This will make tools embedding maven much more difficult to behave consistently with maven.
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
Outdated
Show resolved
Hide resolved
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
Outdated
Show resolved
Hide resolved
Allowing to ovveride dozens of methods makes MavenCli contract vague and could complicate miantenance. going forward It will be easier to maintain MavenCli with public api/interface extracted. For clients it will be easier as well to implement custom logic by delegation. So far it is unclear how to use and extend this class correctly and safely |
The main usage is apache/maven-mvnd@6d2fb40 |
This now matches the returned type better.
@michael-o This is a small refactoring, the goal is mainly to make
mvnd
main class not to rewrite the wholeMavenCli
.The consequence for
mvnd
is in the following commit apache/maven-mvnd@6d2fb40.