-
Notifications
You must be signed in to change notification settings - Fork 225
Bringing Durable Task Java as a Maven module inside the Java SDK #1575
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
base: master
Are you sure you want to change the base?
Conversation
8e649ef to
4e9e636
Compare
|
Merging this PR will mean that we need to archive https://github.com/dapr/durabletask-java/ |
af11647 to
5b6f098
Compare
a05f238 to
72defa8
Compare
Signed-off-by: salaboy <[email protected]>
Signed-off-by: Matheus Cruz <[email protected]> Signed-off-by: salaboy <[email protected]>
* Align Java API with other languages Signed-off-by: Matheus Cruz <[email protected]> * Update documentation Signed-off-by: Matheus Cruz <[email protected]> * Change return type of waitForWorkflowStart method Signed-off-by: artur-ciocanu <[email protected]> --------- Signed-off-by: Matheus Cruz <[email protected]> Signed-off-by: artur-ciocanu <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
1c8269e to
8e842e8
Compare
|
@mcruzdev @artur-ciocanu folks I want you to be aware of this, as the PRs to align versions and remove netty-shaded are all collapsed here. |
|
@dapr/approvers-java-sdk @dapr/maintainers-java-sdk please review and provide feedback. We should not merge other PRs in durabletask-java until this is merged here to avoid divergence. For now, we will need to leave with low code coverage, as we are brining this module to the SDK, it will be easier to add tests as we move forward. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1575 +/- ##
=============================================
- Coverage 76.91% 62.24% -14.68%
- Complexity 1592 1948 +356
=============================================
Files 145 248 +103
Lines 4843 7440 +2597
Branches 562 832 +270
=============================================
+ Hits 3725 4631 +906
- Misses 821 2469 +1648
- Partials 297 340 +43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -0,0 +1,342 @@ | |||
| ///* | |||
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.
If all the other changes are approved I am happy to fix this tests as this follows the same pattern as this PR: #1529 were we remove all these tests and related dependencies in favour of pre made certs. Please leave this file out of the overall review of this PR. 🙏
|
@salaboy this PR LGTM. Should we exclude this module from the coverage ? Otherwise codecov will start failing for every PR. |
Yes, that is something we can do. I really want us to bring the testing level up for this module. |
Description
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1571
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: