Skip to content

Actualize libs versions. Move tests to junit jupiter #284

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

Merged
merged 8 commits into from
Jun 21, 2022

Conversation

altro3
Copy link
Contributor

@altro3 altro3 commented May 8, 2022

Move tests to junit jupiter

Minor libs upgrade:
elasticsearch 7.17.4
jackson 2.13.3
yasson 2.0.4
classgraph 4.8.147
testcontainers 1.17.2

@cla-checker-service
Copy link

cla-checker-service bot commented May 8, 2022

💚 CLA has been signed

@swallez
Copy link
Member

swallez commented May 24, 2022

Thanks for this contribution! Can you please sign the Contributor Agreement so that we can move forward?

@altro3
Copy link
Contributor Author

altro3 commented May 24, 2022

@swallez Hi! I signed it 5 times and already have Csigned ontributor Agreement in pdf format, but cla-checker-service didn't understand it :-(((

@swallez
Copy link
Member

swallez commented May 25, 2022

@altro3 I'll check with our infra team about the CLA to get this sorted out. edit: the CLA finally did get through!

I won't be available until next week. I'll take care of this then.

Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Thanks for this work @altro3.

Testing this PR showed that com.github.jk1.dependency-license-report has to be updated to version 2.0 (1.19 is not compatible). This version 2.0 however requires some additional changes in java-client/build.gradle.kts:SpdxReporter.

Since this PR brings other interesting updates such as Junit5, can you please revert the Gradle upgrade, so that we can handle the SpdxReporter upgrade separately?

I also added some suggestions about some library versions that should either be kept as is (json-p libraries) or upgraded.

@altro3
Copy link
Contributor Author

altro3 commented May 31, 2022

@swallez Hi! Ok, understood. All changes commited

@altro3 altro3 requested a review from swallez June 5, 2022 20:53
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, and thanks for the updates!

However, can you please revert the Gradle update, even if this is a minor version change? This is what I implied with my previous comment, and I apologize if this wasn't clear.

The Gradle files in this PR do not match those I get when upgrading to Gradle 6.9.1 myself. I assume good intentions from your side, but we cannot take any risk updating gradle-wrapper.jar to something we cannot verify. So let's revert it, and I'll work on the upgrade to 7.x.

Thanks for your understanding. Once the Gradle update is reverted (and the nitpick on .gitignore resolved), we will be good to go.

.gitignore Outdated
@@ -16,6 +16,7 @@ gradle-app.setting
# IntelliJ Idea
.idea
*.iml
out
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed as there's no out directory created by the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@swallez ok, fixed

Move tests to junit jupiter
Minor libs upgrade:
elasticsearch 7.17.3
jackson 2.13.2
jakarta.json-api 2.1.0
parsson 1.1.0
yasson 2.0.4
classgraph 4.8.146
testcontainers 1.17.1
altro3 added 2 commits June 21, 2022 17:35
Move tests to junit jupiter
Minor libs upgrade:
elasticsearch 7.17.3
jackson 2.13.2
jakarta.json-api 2.1.0
parsson 1.1.0
yasson 2.0.4
classgraph 4.8.146
testcontainers 1.17.1
Move tests to junit jupiter
Minor libs upgrade:
elasticsearch 7.17.4
jackson 2.13.2
jakarta.json-api 2.1.0
parsson 1.1.0
yasson 2.0.4
classgraph 4.8.146
testcontainers 1.17.2
@swallez
Copy link
Member

swallez commented Jun 21, 2022

@altro3 if this can be of help, here are some commands to revert the changes to Gradle. This is the only thing missing to merge this PR.

git checkout main -- gradlew gradlew.bat gradle
git commit -m 'Restore Gradle'
git push

Alternatively, as a maintainer of the upstream repo I should be able to push changes to your PR, and can handle it from there.

altro3 added 3 commits June 21, 2022 21:14
Minor libs upgrade:
elasticsearch 7.17.4
jackson 2.13.2
jakarta.json-api 2.1.0
parsson 1.1.0
yasson 2.0.4
classgraph 4.8.146
testcontainers 1.17.2
Minor libs upgrade:
elasticsearch 7.17.4
jackson 2.13.2
jakarta.json-api 2.1.0
parsson 1.1.0
yasson 2.0.4
classgraph 4.8.146
testcontainers 1.17.2
@altro3
Copy link
Contributor Author

altro3 commented Jun 21, 2022

@swallez done! :-)

Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for this great contribution @altro3, and also for keeping up with my requests 😉

@swallez
Copy link
Member

swallez commented Jun 21, 2022

I also took the liberty of adding a commit to your PR (9b1c0ce) to update the part of the build system that produces a license report. There are many variations used to name the EPL-2.0...

swallez added a commit that referenced this pull request Jun 21, 2022
swallez added a commit that referenced this pull request Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants