-
Notifications
You must be signed in to change notification settings - Fork 13
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
[MRRESOURCES-102] Implement MRRESOURCES-102 #2
Conversation
9a02538
to
6df9986
Compare
src/main/java/org/apache/maven/plugin/resources/remote/ProcessRemoteResourcesMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugin/resources/remote/ProcessRemoteResourcesMojo.java
Outdated
Show resolved
Hide resolved
{ | ||
MavenFileFilterRequest req = new MavenFileFilterRequest(); | ||
req.setFrom( source ); | ||
req.setTo( file ); | ||
req.setFiltering( isFiltering ); | ||
req.setFiltering( true ); |
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.
But this filtering always active, is that what we want?
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.
The property useResourceFiltering
is still adhered to. It's evaluated on line 1279.
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.
@rfscholte do we need more review here?
…-102 # Conflicts: # src/main/java/org/apache/maven/plugin/resources/remote/ProcessRemoteResourcesMojo.java
This is out of sync with master and doesn't have tests. I'm inclined to close this unless someone wants to pick it up. |
Yeah, I know 😕 It was suggested on the mailing list to add unit tests. But the current tests were old (Maven 2-based). So the suggestion was to first fix MRRESOURCES-92. That posed too big of a hurdle for me. It seems MRRESOURCES-92 still isn't fixed. It's auto-closed because it was stale. So this project will probably not get much pull request effort from outsiders until that's done. (Funny enough there were pull requests merged without tests... Guess I'm just not that lucky 😀) |
This implements MRRESOURCES-102 by using Maven resource filtering.