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-18499] Update the microprofile-rest-client for the Common Enhan… #780

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

jamezp
Copy link
Member

@jamezp jamezp commented Nov 22, 2023

…cements. Refactor the quickstart to a single module with a test for the MicroProfile REST Client.

https://issues.redhat.com/browse/WFLY-18499

@jamezp jamezp force-pushed the WFLY-18499 branch 2 times, most recently from 194a3a0 to 59dba33 Compare November 22, 2023 22:54
Copy link
Contributor

@emmartins emmartins left a comment

Choose a reason for hiding this comment

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

Thanks a lot for you major rework, functionality all is good, there are a few changes needed tho, mostly about the README.adoc.

microprofile-rest-client/pom.xml Show resolved Hide resolved
microprofile-rest-client/pom.xml Outdated Show resolved Hide resolved
<groupId>org.wildfly.plugins</groupId>
<artifactId>wildfly-maven-plugin</artifactId>
<version>${version.plugin.wildfly}</version>
<configuration>
Copy link
Contributor

Choose a reason for hiding this comment

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

considering that we won't have provisioned-server profile (see below), this config is no longer needed

Copy link
Member Author

Choose a reason for hiding this comment

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

If we remove this, then it can't be used for development. Why not just keep it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, this is needed for the openshift profile.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant only the configuration part, not the plugin management itself, the openshift profile uses full config.

microprofile-rest-client/pom.xml Show resolved Hide resolved
microprofile-rest-client/pom.xml Show resolved Hide resolved
microprofile-rest-client/README.adoc Show resolved Hide resolved
microprofile-rest-client/README.adoc Show resolved Hide resolved
microprofile-rest-client/README.adoc Outdated Show resolved Hide resolved
@jamezp jamezp force-pushed the WFLY-18499 branch 3 times, most recently from 1410d49 to 8dc74d4 Compare November 27, 2023 16:52
Copy link
Contributor

@emmartins emmartins left a comment

Choose a reason for hiding this comment

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

From what it looks like there is only a change needed on the share-docs, where I believe you removed too much.

<groupId>org.wildfly.plugins</groupId>
<artifactId>wildfly-maven-plugin</artifactId>
<version>${version.plugin.wildfly}</version>
<configuration>
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant only the configuration part, not the plugin management itself, the openshift profile uses full config.

microprofile-rest-client/pom.xml Show resolved Hide resolved
…cements. Refactor the quickstart to a single module with a test for the MicroProfile REST Client.

Signed-off-by: James R. Perkins <[email protected]>
@emmartins emmartins merged commit 51d3a8a into wildfly:main Nov 30, 2023
19 checks passed
@emmartins
Copy link
Contributor

There are a few issues on README that I only noticed post merge, I will send a PR wrt that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants