-
Notifications
You must be signed in to change notification settings - Fork 267
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
Fix for issue #582 - update-properties does not work across parent-child pom #616
Conversation
@slawekjaranowski do you think you can find some time to look into this? |
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.
- it looks like we can add a unit tests for PomHelper, it should be easier to check coverage by test
- what will be if in parent we will have dependencyManagment and in child dependency without version, and property
- what will be if in parent we will have dependencyManagment and in child no dependency, but only property
disclosure - I'm not know this code ... and logic looks quite complex, so will be good if someone else looks at it
The change in logic is actually quite simple, yet for some reason github diff messed up. If you hide whitespaces changes the diff looks a lot better - the changes are highlighted correctly. The logic of dependencyManagement vs property stays the same as for a single pom. The only change there is to actually iterate over the parent poms and process those as well when looking for properties. @olamy @bmarwell I've seen you made changes to PomHelper more recently. Do you guys think you can look into it? |
please change commit message to something more related what we do, eg like:
pleas also squash to one commit |
@slawekjaranowski done |
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.
ok, looks ok for me.
I will wait a few days for other comments.
@slawekjaranowski Can this be merged now since there are no more comments? Is there a timeframe for the next release of the plugin? Anything that I need to do for the release? |
@slawekjaranowski It has been more than a week and there are no comments. Could we merge this PR? |
Ok. |
Fixes mojohaus#582 o Updated PomHelper to validate version properties also across parents.
@slawekjaranowski |
@prodj17 - thanks |
Fixed #582
o Updated PomHelper to validate version properties also across parents.