Always preserve dominant node value (even if empty) by kwin · Pull Request #217 · codehaus-plexus/plexus-utils
Navigation Menu
{{ message }}
codehaus-plexus / plexus-utils Public
- Notifications You must be signed in to change notification settings
- Fork 44
Merged
michael-o merged 1 commit intocodehaus-plexus:masterfrom
Oct 15, 2022Merged
Conversation
Copy link
Contributor Author
kwin
commented
Sep 12, 2022
kwin commented
Sep 12, 2022I tested this against the Maven Integration Tests and there were no failures.
Copy link
Member
michael-o
commented
Oct 10, 2022
michael-o commented
Oct 10, 2022Can you run this change against all Maven Plugin Tools ITs?
michael-o approved these changes Oct 10, 2022
Copy link
Member
michael-o
left a comment
•
edited
Loading
michael-o
left a comment
•
edited
Loading
edited
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This make sense. Requirement for merge is that MPT are running fine. Waiting for confirmation then I can merge.
michael-o
marked this pull request as ready for review
Copy link
Contributor Author
kwin
commented
Oct 10, 2022
kwin commented
Oct 10, 2022Can you run this change against all Maven Plugin Tools ITs?
You mean https://github.com/apache/maven-plugin-tools being build with Prun-its after referencing this PR?
Copy link
Member
michael-o
commented
Oct 10, 2022
michael-o commented
Oct 10, 2022Can you run this change against all Maven Plugin Tools ITs?
You mean https://github.com/apache/maven-plugin-tools being build with
Prun-itsafter referencing this PR?
Correct, this code must be in use for these ITs.
Copy link
Member
michael-o
commented
Oct 10, 2022
michael-o commented
Oct 10, 2022@kwin Please rebase against master.
Copy link
Contributor Author
kwin
commented
Oct 10, 2022
kwin commented
Oct 10, 2022@michael-o Actually this was thought as an alternative to #213 (as this includes blank values), but we can also obviously apply it on top.
Copy link
Member
michael-o
commented
Oct 10, 2022
michael-o commented
Oct 10, 2022@michael-o Actually this was thought as an alternative to #213 (as this includes blank values), but we can also obviously apply it on top.
I see it as a compliment for #213 therefore a rebase is required.
kwin
force-pushed
the
bugfix/preserve-dominant-node-value-even-if-empty
branch
2 times, most recently
from
f112155 to
4f5e2f2
Compare
Copy link
sonatype-lift
bot
commented
Oct 11, 2022
sonatype-lift bot commented
Oct 11, 2022⚠️ 22 God Classes were detected by Lift in this project. Visit the Lift web console for more details.
Copy link
Contributor Author
kwin
commented
Oct 11, 2022
kwin commented
Oct 11, 2022Build is successful with Prun-itsof https://github.com/apache/maven-plugin-tools after upgrading to this version of plexus-utils.
TBH I don't think there is an IT for this config merging behaviour in MPT.
michael-o pushed a commit to kwin/plexus-utils that referenced this pull request
Oct 15, 2022…in case it is empty (codehaus-plexus#216) This codehaus-plexus#216 and fixes codehaus-plexus#217
michael-o
force-pushed
the
bugfix/preserve-dominant-node-value-even-if-empty
branch
from
4f5e2f2 to
1d660b5
Compare
…in case it is empty (codehaus-plexus#216) This fixes codehaus-plexus#216 and closes codehaus-plexus#217
michael-o
force-pushed
the
bugfix/preserve-dominant-node-value-even-if-empty
branch
from
1d660b5 to
67ac243
Compare
michael-o
merged commit
67ac243
into
codehaus-plexus:master
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment