Skip to content

Replace reactive programming library #16

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
Mar 18, 2019
Merged

Replace reactive programming library #16

merged 8 commits into from
Mar 18, 2019

Conversation

lpatino10
Copy link
Contributor

@lpatino10 lpatino10 commented Mar 14, 2019

Related to watson-developer-cloud/java-sdk#1013

This PR replaces the current reactive programming library (https://mvnrepository.com/artifact/org.glassfish.jersey.bundles.repackaged/jersey-jsr166e) with RxJava.

This library was chosen because it is:

  • popular among Java developers
  • used in high-profile projects
  • currently maintained/updated
  • Java 7-compatible

Tests have been added in this PR to ensure the two new execution method (reactiveRequest()) work s properly, while documentation has been updated and is currently in review in the Watson SDK repo: watson-developer-cloud/java-sdk#1045

Along with the above changes, both executeWithDetails() and enqueueWithDetails() have been removed. The normal version of those methods now return the same wrapped HTTP response object, so that we always return the IBM Cloud API response + extra info.

BREAKING CHANGE: The rx() and rxWithDetails() methods have been removed. This type of functionality
should be replaced with the new reactiveRequest() and reactiveRequestWithDetails() methods
padamstx
padamstx previously approved these changes Mar 14, 2019
Copy link
Member

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

LGTM
I can't offer much insight into this one, so I hope you know what you're doing :)

@germanattanasio
Copy link
Contributor

I think we want to default to detailed responses so maybe one of the reactive responses can be eliminated.

@lpatino10
Copy link
Contributor Author

Ah yeah I forgot about that actually. I'll go ahead and do that in this PR then for all versions.

BREAKING CHANGE: The old executeWithDetails() and enqueueWithDetails() methods have been deleted. In
addition, the basic versions of those methods now return the wrapped response that the old methods
used to.
Copy link
Contributor

@germanattanasio germanattanasio left a comment

Choose a reason for hiding this comment

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

Looks good 🙌
Great job!

@lpatino10 lpatino10 merged commit c5a5ef5 into master Mar 18, 2019
@lpatino10 lpatino10 deleted the new-async-lib branch March 18, 2019 17:04
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.

3 participants