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

#367: Include parent projects in property resolution #759

Merged
merged 1 commit into from Oct 17, 2022
Merged

#367: Include parent projects in property resolution #759

merged 1 commit into from Oct 17, 2022

Conversation

jarmoniuk
Copy link
Contributor

Includes parent projects in property resolution for property updates. Instead of only considering the model of the current project, will also consult parent models to see if there are property definitions for properties defined in the current project.

@jarmoniuk jarmoniuk force-pushed the issue-367-properties-from-parent branch from 9362da2 to fbf98b5 Compare October 15, 2022 11:12
@jarmoniuk
Copy link
Contributor Author

@slawekjaranowski Please review.

@jarmoniuk jarmoniuk force-pushed the issue-367-properties-from-parent branch from fbf98b5 to 354eef0 Compare October 15, 2022 16:58
@jarmoniuk jarmoniuk mentioned this pull request Oct 15, 2022
Closed
@slawekjaranowski slawekjaranowski added this to the 2.13.0 milestone Oct 15, 2022
@slawekjaranowski slawekjaranowski linked an issue Oct 15, 2022 that may be closed by this pull request
Ignored usage of properties in child POMs #367
Closed
@jarmoniuk jarmoniuk force-pushed the issue-367-properties-from-parent branch from 354eef0 to 1d55c7c Compare October 16, 2022 16:16
@jarmoniuk jarmoniuk force-pushed the issue-367-properties-from-parent branch from 1d55c7c to 9605f58 Compare October 16, 2022 17:06
@jarmoniuk
Copy link
Contributor Author

As you can see, tried that, but it caused the it-encoding-001 test to fail. Files.newBufferedReader assumes UTF-8 for input, which is not always the case.

@slawekjaranowski slawekjaranowski merged commit bce8646 into mojohaus:master Oct 17, 2022
@jarmoniuk jarmoniuk deleted the issue-367-properties-from-parent branch October 17, 2022 08:29
@madorb
Copy link

madorb commented Oct 27, 2022 •
edited

well this was an unexpected (unwanted) surprise! is there any way to disable this functionality?

consider users of say, spring boot that have spring-boot-dependencies as their parent

it's quite unhelpful to now see the full list of all properties from the parent by default. Most of those are for dependencies any given application doesn't use.

I think the functionality itself is a great thing to have, and is important for lots of use cases - but if you're using a 3rd party BOM as a parent it's a big step backwards in helpfulness. This radical of a change really should come with a configuration option either in the xml or command line.

@jarmoniuk
Copy link
Contributor Author

@madorb yes, but only in the upcoming release, see #795

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

@slawekjaranowski slawekjaranowski slawekjaranowski left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.13.0
Development

Successfully merging this pull request may close these issues.

Ignored usage of properties in child POMs
3 participants
@jarmoniuk @madorb @slawekjaranowski