-
Notifications
You must be signed in to change notification settings - Fork 189
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
Add soap dataformat quarkus extension #883
Conversation
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.
Thanks @mmelko, some comments inline.
integration-tests/soap/pom.xml
Outdated
<configuration> | ||
<systemProperties> | ||
<native.image.path>${project.build.directory}/${project.build.finalName}-runner</native.image.path> | ||
</systemProperties> | ||
</configuration> |
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.
<configuration> | |
<systemProperties> | |
<native.image.path>${project.build.directory}/${project.build.finalName}-runner</native.image.path> | |
</systemProperties> | |
</configuration> |
The config can be removed.
## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
## See the License for the specific language governing permissions and | ||
## limitations under the License. | ||
## --------------------------------------------------------------------------- |
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.
Empty application.properties can be removed.
assertThat(resp).contains("<name>" + msg + "</name>"); | ||
assertThat(resp).contains("<ns2:Envelope xmlns:ns2=\"http://schemas.xmlsoap.org/soap/envelope/\""); | ||
|
||
System.out.println(resp); |
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.
Could you please remove the System.out.println ?
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 last issue from my PoV.
...ration-tests/soap/src/main/java/org/apache/camel/quarkus/component/soap/it/SoapResource.java
Show resolved
Hide resolved
...ration-tests/soap/src/main/java/org/apache/camel/quarkus/component/soap/it/SoapResource.java
Show resolved
Hide resolved
extensions/soap/runtime/pom.xml
Outdated
|
||
<properties> | ||
<firstVersion>1.0.0-M5</firstVersion> | ||
<saaj.version>1.5.0</saaj.version> |
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.
Is the saaj.version
version property used anywhere?
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.
I forgot to remove this. Thanks!
a0bc750
to
1bfe870
Compare
1bfe870
to
0db4448
Compare
<dependency>
<groupId>javax.xml.ws</groupId>
<artifactId>jaxws-api</artifactId>
</dependency> Had to be added to work this with java 11. Now I'm waiting for CI results |
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.
We'll need to tweak the <firstVersion>
as this wont have made 1.0.0-M5.
We can do that after merging though.
Thanks, @mmelko ! |
fix #763