Skip to content

[MNG-8494] Restore Maven 3 compatibility#2031

Merged
gnodet merged 5 commits intoapache:masterfrom
gnodet:m3-full-compat
Jan 10, 2025
Merged

[MNG-8494] Restore Maven 3 compatibility#2031
gnodet merged 5 commits intoapache:masterfrom
gnodet:m3-full-compat

Conversation

@gnodet
Copy link
Contributor

@gnodet gnodet commented Jan 7, 2025

No description provided.

elharo
elharo reviewed Jan 8, 2025
View reviewed changes
api/maven-api-model/src/main/mdo/maven.mdo Outdated
<codeSegment>
<version>4.0.0/4.0.99</version>
<code>
<![CDATA[
Copy link
Contributor

@elharo elharo Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what CDATA is doing here. It's not needed

api/maven-api-toolchain/src/main/mdo/toolchains.mdo Outdated
<codeSegment>
<version>1.0.0/1.1.0</version>
<code>
<![CDATA[
Copy link
Contributor

@elharo elharo Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what CDATA is doing here. It's not needed

...aven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java

protected AbstractMavenTransferListener(MessageBuilderFactory messageBuilderFactory, PrintWriter out) {
this.messageBuilderFactory = messageBuilderFactory;
protected PrintStream out;
Copy link
Contributor

@elharo elharo Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PrintStream is wonky for multiple reasons. Can we just use an output stream writer instead?

Copy link
Contributor Author

@gnodet gnodet Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully agree, however the code has already been cleaned up in the non deprecated version:

maven/impl/maven-cli/src/main/java/org/apache/maven/cling/transfer/AbstractMavenTransferListener.java

Line 36 in 698614e

public abstract class AbstractMavenTransferListener extends AbstractTransferListener {

That class is purely for compatibility, so no protected stuff should be changed.

The maven-embedder is here for compatibility and not used when launching Maven from CLI directly.

compat/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelCache.java
/**
* Puts the specified data into the cache.
*
* @param groupId The group id of the cache record, must not be {@code null}.
Copy link
Contributor

@elharo elharo Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oracle javadoc guidelines are no initial capitalization on Javadoc comments and period only after or before a complete sentence

compat/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelCacheTag.java

/**
* Creates a copy of the data suitable for retrieval from the cache. The retrieved data can be mutated after the
* cache is queried but the state of the cache must not change so we need to make a copy.
Copy link
Contributor

@elharo elharo Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

queried,

@cstamas cstamas added this to the 4.0.0-rc-3 milestone Jan 8, 2025
@gnodet
Copy link
Contributor Author

gnodet commented Jan 8, 2025

99% of the code here is copy/pasted from maven-3.9.x branch to restore compatibility... so I'm fine with any change you suggest, but I really don't expect you to closely review all the changed files.

@cstamas
Copy link
Member

cstamas commented Jan 8, 2025

These are literally copies of Maven 3 classes.

@cstamas
Copy link
Member

cstamas commented Jan 9, 2025 •
edited
Loading

IMHO last commit should be undone:

@gnodet gnodet merged commit 5cef91e into apache:master Jan 10, 2025
@jira-importer
Copy link

jira-importer commented Jul 7, 2025

Resolve #9943

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elharo elharo elharo left review comments

@cstamas cstamas cstamas approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0.0-rc-3

Development

Successfully merging this pull request may close these issues.

4 participants

@gnodet @cstamas @jira-importer @elharo