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

NXP-28726: bump woodstox library version #4097

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

NourNuxeo
Copy link
Contributor

No description provided.

@NourNuxeo NourNuxeo requested a review from a team as a code owner May 29, 2020 17:23
@nuxeojenkins
Copy link
Collaborator

View issue in JIRA: NXP-28726: Fix woodstox library version used by add nuxeo-management-rest-api 1.0.0

@ghost ghost requested review from bdelbosc and kevinleturc and removed request for a team May 29, 2020 17:24
@NourNuxeo NourNuxeo marked this pull request as draft May 30, 2020 12:50
@NourNuxeo NourNuxeo force-pushed the fix-NXP-28726-bump-woodstox-library-version branch 3 times, most recently from 62dd690 to 9a0ac6e Compare June 1, 2020 18:34
@NourNuxeo NourNuxeo marked this pull request as ready for review June 1, 2020 18:35
efge
efge previously approved these changes Jun 1, 2020
@NourNuxeo NourNuxeo force-pushed the fix-NXP-28726-bump-woodstox-library-version branch 2 times, most recently from b7d76e2 to 776f2f1 Compare June 2, 2020 17:11
Comment on lines 83 to 90

<!-- excluded from chemistry-opencmis-commons-impl for groupId & version issues -->
<dependency>
<groupId>com.fasterxml.woodstox</groupId>
<artifactId>woodstox-core</artifactId>
<scope>test</scope>
</dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

efge
efge previously approved these changes Jun 3, 2020
Comment on lines 84 to 89
<!-- excluded from chemistry-opencmis-commons-impl for groupId & version issues -->
<dependency>
<groupId>com.fasterxml.woodstox</groupId>
<artifactId>woodstox-core</artifactId>
<scope>test</scope>
</dependency>
Copy link
Member

@troger troger Jun 8, 2020

Choose a reason for hiding this comment

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

I'm not sure this is really needed, the dependency added to nuxeo-opencmis-impl should be enough (and you don't have it on the 10.10 PR 😃 )

It should be tested again without it.

Copy link
Contributor Author

@NourNuxeo NourNuxeo Jun 8, 2020

Choose a reason for hiding this comment

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

The list of failing tests was not exactly the same for Master, there is one extra test that was failing because it was missing the dependency (so I scoped it accordingly):

Copy link
Member

Choose a reason for hiding this comment

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

The server is returning 500, I don't see why adding a test dependency to the server ftests would fix the issue... 🤔

Is it possible to relaunch it w/o the dep so that we can see the server logs? :)

Copy link
Contributor Author

@NourNuxeo NourNuxeo Jun 9, 2020

Choose a reason for hiding this comment

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

Sure
I hope I can pass through the randoms !
(but about the 500 error, since it's a test involving CMIS on a server which fails, I'm not surprized to have a 500 though)

Copy link
Member

@troger troger Jun 9, 2020

Choose a reason for hiding this comment

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

The error is server side, so it's related to jars in the server. Here you are adding a dependency to a functional tests module, it won't end up in the server.

Adding the dependency to the nuxeo-opencmis-impl module should be enough as this module (and its dependencies) will be deployed in the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NourNuxeo NourNuxeo force-pushed the fix-NXP-28726-bump-woodstox-library-version branch from 776f2f1 to ab9e8b8 Compare June 9, 2020 10:51
@NourNuxeo NourNuxeo force-pushed the fix-NXP-28726-bump-woodstox-library-version branch from ab9e8b8 to 73a2475 Compare June 9, 2020 17:03
@efge
Copy link
Member

efge commented Jun 9, 2020

Ok so in the end the code is the same as for 10.10 #4096

@NourNuxeo NourNuxeo merged commit cf22cda into master Jun 9, 2020
@NourNuxeo NourNuxeo deleted the fix-NXP-28726-bump-woodstox-library-version branch June 9, 2020 21:07
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.

5 participants