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

Using option otherPromise instead of sync #388

Merged
merged 1 commit into from Aug 6, 2016
Merged

Using option otherPromise instead of sync #388

merged 1 commit into from Aug 6, 2016

Conversation

Ritzlgrmft
Copy link
Contributor

Since the plugin's methods return already a promise, the workaround with the sync option was used. This worked well for the method calls. However, when either cordova or the plugin was not available, an error was thrown, instead of rejecting the returned promise.

Therefore a better way is to use the otherPromise option, introduced with https://github.com/driftyco/ionic-native/releases/tag/v1.3.8.

Since the plugin's methods return already a promise, the workaround with the `sync` option was used. This worked well for the method calls. However, when either cordova or the plugin was not available, an error was thrown, instead of rejecting the returned promise.

Therefore a better way is to use the `otherPromise` option, introduced with https://github.com/driftyco/ionic-native/releases/tag/v1.3.8.
@mlynch
Copy link
Collaborator

mlynch commented Aug 4, 2016

Thanks, will have ibrahim review for the next release

@ihadeed
Copy link
Collaborator

ihadeed commented Aug 4, 2016

Looks good @Ritzlgrmft, have you tested it?

@Ritzlgrmft
Copy link
Contributor Author

Ritzlgrmft commented Aug 5, 2016

Yes, it is working.

Additionally, I get a rejected promise, if the plugin is not available - as it should be.

@ihadeed ihadeed merged commit 306cb5d into danielsogl:master Aug 6, 2016
@ihadeed
Copy link
Collaborator

ihadeed commented Aug 6, 2016

thanks @Ritzlgrmft

@Ritzlgrmft Ritzlgrmft deleted the patch-1 branch August 26, 2016 06:23
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.

None yet

3 participants