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.

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

[WFLY-15092] Use maven-resource-plugin filtering to avoid duplicating … #14507

Merged
merged 1 commit into from Aug 10, 2021

Conversation

bstansberry
Copy link
Contributor

@bstansberry bstansberry commented Jul 23, 2021

@bstansberry bstansberry added the hold PR should not be merged for some reason. label Jul 23, 2021
@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Jul 23, 2021
<!-- Use filter expressions so standard WildFly and WildFly Preview can use artifacts with different qualifiers -->
<artifact name="@gav.exp.start@com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider@module.jakarta.classifier@@gav.exp.end@"/>
<artifact name="@gav.exp.start@com.fasterxml.jackson.jaxrs:jackson-jaxrs-base@module.jakarta.classifier@@gav.exp.end@"/>
<artifact name="@gav.exp.start@com.fasterxml.jackson.module:jackson-module-jaxb-annotations@module.jakarta.classifier@@gav.exp.end@"/>
Copy link
Contributor

@yersan yersan Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possible alternative to allow the nested expressions avoiding the uses of @gav.exp.start@ and @gav.exp.end@ is to configure a scape character on the maven resource plugin. If we scape the first $, then it seems we could use nested expressions like the following one:

<artifact name="\${com.fasterxml.jackson.jaxrs:jackson-jaxrs-json-provider${module.jakarta.classifier}}"/>

The maven resource plugin has to include the following configuration:

<plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-resources-plugin</artifactId>
        <version>3.2.0</version>
        <configuration>
          ...
          <escapeString>\</escapeString>
          ...
        </configuration>
</plugin>

It is not a great advantage, but personally, it improves the readability of the module from my point of view and removes two maven properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yersan ; I've updated to use that.

@bstansberry bstansberry force-pushed the jakarta_module_filter branch 2 times, most recently from 8b941dc to 5c32d92 Compare August 6, 2021 22:10
@bstansberry bstansberry marked this pull request as ready for review August 6, 2021 23:24
@bstansberry
Copy link
Contributor Author

@jamezp or @ronsigal Is this approach to configuring this module ok with you?

@bstansberry bstansberry changed the title Prototype using maven-resource-plugin filtering to avoid duplicating … [WFLY-15092] Use maven-resource-plugin filtering to avoid duplicating … Aug 8, 2021
@bstansberry bstansberry removed the hold PR should not be merged for some reason. label Aug 8, 2021
@jamezp
Copy link
Member

jamezp commented Aug 9, 2021

@jamezp or @ronsigal Is this approach to configuring this module ok with you?

@bstansberry looks good to me.

@bstansberry bstansberry merged commit 3012da9 into wildfly:main Aug 10, 2021
@bstansberry bstansberry deleted the jakarta_module_filter branch August 10, 2021 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
3 participants