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

#815 Upgrade and fix issues for new Reactor version. #828

Closed
wants to merge 5 commits into from

Conversation

MatejNedic
Copy link
Contributor

@MatejNedic MatejNedic commented Feb 2, 2023

Description

Solves #815

Issue reference

#815

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

fix checkstyle

add missing space

Initial commit
@MatejNedic MatejNedic requested review from a team as code owners February 2, 2023 19:17
artursouza and others added 2 commits February 2, 2023 12:30

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@artursouza artursouza left a comment

Choose a reason for hiding this comment

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

Upgrading it breaks the integration tests. There might be more dependencies to update because of this.

@MatejNedic
Copy link
Contributor Author

Will upgrade to spring boot 3.0.2 and see how it will go.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…ctor-core
@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #828 (5c1bd44) into master (a947d5d) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 5c1bd44 differs from pull request most recent head 5cc1af3. Consider uploading reports for the commit 5cc1af3 to get more accurate results

@@             Coverage Diff              @@
##             master     #828      +/-   ##
============================================
- Coverage     77.92%   77.91%   -0.02%     
  Complexity     1260     1260              
============================================
  Files           116      116              
  Lines          3892     3889       -3     
  Branches        458      458              
============================================
- Hits           3033     3030       -3     
  Misses          627      627              
  Partials        232      232              
Impacted Files Coverage Δ
sdk/src/main/java/io/dapr/client/DaprHttp.java 92.30% <ø> (ø)
.../java/io/dapr/internal/opencensus/GrpcWrapper.java 86.48% <ø> (ø)
...ain/java/io/dapr/actors/client/DaprGrpcClient.java 93.54% <100.00%> (ø)
...k/src/main/java/io/dapr/client/DaprClientGrpc.java 89.00% <100.00%> (-0.07%) ⬇️
...k/src/main/java/io/dapr/client/DaprClientHttp.java 86.89% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@champel
Copy link
Contributor

champel commented Feb 3, 2023

I created #830, that allows to use reactor only upgrading this dependency. In order to get closer to Spring Boot 3, i also propose #831 to upgrade to the latest version 2 of Spring boot. Both PR are passing integration tests and validation in my account (https://github.com/champel/dapr-java-sdk/pulls). @MatejNedic and me are working in a project where we have migrated to Spring Boot 3 but we want still to use the awesome dapr.

@MatejNedic
Copy link
Contributor Author

MatejNedic commented Feb 3, 2023

@champel from what I have seen to bump version of spring boot 3.0 in this PR it will require changing some code since with Java 17 I had build issues inside this repository.

@champels
Copy link

champels commented Feb 3, 2023

@MatejNedic , the pipeline is building with versions 11,13,15 and 16. So I downgraded my local version to 11 to build the sdk.

@MatejNedic
Copy link
Contributor Author

Closing in favour of #830

@MatejNedic MatejNedic closed this Feb 7, 2023
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.

4 participants