Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[MNG-7862] The ModelLocator should always be used when locating pom.xml #1217

Merged
merged 2 commits into from
Aug 23, 2023
Merged

[MNG-7862] The ModelLocator should always be used when locating pom.xml #1217

merged 2 commits into from
Aug 23, 2023

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Aug 21, 2023 •
edited

JIRA issue: https://issues.apache.org/jira/browse/MNG-7862
IT PR: apache/maven-integration-testing#289

There are a few places where the pom.xml file name is hardcoded. There's a known interface to locate those, so those references should be cleaned.
One IT has to be adjusted (see PR) because the default value in the model is changed from ../pom.xml to .. for the parent relativePath default value.

@gnodet gnodet force-pushed the MNG-7862-fix-model-locator branch 2 times, most recently from d4c25f4 to 2da261f Compare August 21, 2023 14:25
elharo
elharo reviewed Aug 22, 2023
View reviewed changes
Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

Is this for v4 only? Some of the API changes look incompatible.

maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java Outdated Show resolved Hide resolved
maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java Outdated Show resolved Hide resolved
maven-model-builder/src/main/java/org/apache/maven/model/building/ModelSource3.java Outdated Show resolved Hide resolved
@gnodet
Copy link
Contributor Author

gnodet commented Aug 22, 2023

Is this for v4 only? Some of the API changes look incompatible.

Yes, I did not really envision to backport it. I think most of the changed methods are new in 4.0. The exceptions are the default value in the model and the new ModelSource3 which I introduced to avoid adding a method to an interface that could be possibly implemented by plugins / extensions. I may have missed a possible incompatible change if a plugin/extension actually implements ModelSource2 (as I should provide the previous behaviour in such a case).

@gnodet gnodet requested a review from elharo August 23, 2023 08:02
@elharo
Copy link
Contributor

elharo commented Aug 23, 2023

Probably unrelated but why in the CI output do I see :"Sending email to: rishi@ca.ibm.com"?

@gnodet
Copy link
Contributor Author

gnodet commented Aug 23, 2023

Probably unrelated but why in the CI output do I see :"Sending email to: rishi@ca.ibm.com"?

Not sure, I've disabled the s390x pipeline as it does not take into account the fact that a given branch should be tested with the maven-integration-testing branch with the same name if it exists.

@elharo
Copy link
Contributor

elharo commented Aug 23, 2023

Filed https://issues.apache.org/jira/browse/MNG-7863?filter=-2

@elharo
Copy link
Contributor

elharo commented Aug 23, 2023

OK, looks good overall. Is it possible to retrigger the full CI so we can see that S390 is no longer included?

@gnodet gnodet requested a review from elharo August 23, 2023 16:02
@gnodet
Copy link
Contributor Author

gnodet commented Aug 23, 2023

@elharo could you have another look ?

@gnodet gnodet merged commit f2593b9 into apache:master Aug 23, 2023
18 checks passed
@gnodet gnodet deleted the MNG-7862-fix-model-locator branch September 11, 2023 07:42
@gnodet gnodet added this to the 4.0.0-alpha-8 milestone Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elharo elharo elharo approved these changes

@michael-o michael-o Awaiting requested review from michael-o

@cstamas cstamas Awaiting requested review from cstamas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.0.0-alpha-8
2 participants
@gnodet @elharo