Skip to content

Don't overwrite blank (but non-empty) dominant values during mergeXpp3Dom#213

Merged
michael-o merged 1 commit intocodehaus-plexus:masterfrom
kwin:bugfix/dominant-blank-values-overwritten-during-merge
Oct 10, 2022
Merged

Don't overwrite blank (but non-empty) dominant values during mergeXpp3Dom#213
michael-o merged 1 commit intocodehaus-plexus:masterfrom
kwin:bugfix/dominant-blank-values-overwritten-during-merge

Conversation

@kwin
Copy link
Contributor

@kwin kwin commented Sep 11, 2022 •
edited
Loading

Compare with #212

@kwin kwin mentioned this pull request Sep 11, 2022
Closed
@kwin kwin changed the title Add test exposing that dominant blank (but non-empty) values are Don't overwrite blank (but non-empty) dominant values during mergeXpp3Dom Sep 11, 2022
@kwin kwin marked this pull request as ready for review September 11, 2022 10:55
michael-o
michael-o requested changes Sep 11, 2022
View reviewed changes
@michael-o
Copy link
Member

michael-o commented Sep 11, 2022

I wonder what will fail with current downstream consumer tests

@kwin
Copy link
Contributor Author

kwin commented Sep 11, 2022

I wonder what will fail with current downstream consumer tests

Any particular tests you have in mind? I will try to run Maven ITs against this change.

@michael-o
Copy link
Member

michael-o commented Sep 11, 2022

I wonder what will fail with current downstream consumer tests

Any particular tests you have in mind? I will try to run Maven ITs against this change.

Run this patched version against Core plugins ITs.

@michael-o
Copy link
Member

michael-o commented Sep 11, 2022

I'd like to run tests on plugins. Does it suffice to update Maven Core with this patch?

@kwin
Copy link
Contributor Author

kwin commented Sep 11, 2022 •
edited
Loading

I modified Maven master to define
<plexusUtilsVersion>3.4.3-SNAPSHOT</plexusUtilsVersion> in the reactor pom.xml, build it and ran the Maven Core integration tests successfully against that distro.

@michael-o
Copy link
Member

michael-o commented Sep 11, 2022

I modified Maven master to define <plexusUtilsVersion>3.4.3-SNAPSHOT</plexusUtilsVersion> in the reactor pom.xml, build it and ran the Maven Core integration tests successfully against that distro.

Good, I will repeat with several core plugin ITs with 3.9.0-SNAPSHOT tomorrow.

Will this PR completely cover the downstream Maven issue?

@kwin
Copy link
Contributor Author

kwin commented Sep 11, 2022

Will this PR completely cover the downstream Maven issue?

No, it just allows to apply a workaround by giving a blank value in the plugin configuration to disable the default handling (FTR: the downstream issue is https://issues.apache.org/jira/browse/MNG-6434).

@michael-o michael-o force-pushed the bugfix/dominant-blank-values-overwritten-during-merge branch from ce417d5 to 748933c Compare October 10, 2022 13:18
@michael-o michael-o self-requested a review October 10, 2022 13:19
@sonatype-lift
Copy link

sonatype-lift bot commented Oct 10, 2022

⚠️ 22 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

@michael-o michael-o merged commit 748933c into codehaus-plexus:master Oct 10, 2022
@kwin kwin mentioned this pull request Oct 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kwin @michael-o