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

BREAKING CHANGE: Use promises instead of callbacks #56

Merged
merged 16 commits into from
May 1, 2020
Merged

BREAKING CHANGE: Use promises instead of callbacks #56

merged 16 commits into from
May 1, 2020

Conversation

eliaslecomte
Copy link
Collaborator

Use a promise instead of the callback mechanism. Readme + typescript definition also updated.

@Rapsssito
Copy link
Collaborator

LGTM. However, this will be a breaking change. Should we add both options for backwards compatibility or we release the 4.0.0?

@eliaslecomte
Copy link
Collaborator Author

Maybe tag this and wait until we have a few more breaking changes? Supporting both would be a pain in the ass?

@eliaslecomte
Copy link
Collaborator Author

There are 3 or 4 more callbacks that I would like to replace with promises .

@Rapsssito
Copy link
Collaborator

We can turn this PR in an overall library promisification.

@Rapsssito Rapsssito changed the title Feature: use a promise for isRemoveWifiNetwork BREAKING CHANGE: Use promises instead of callbacks Mar 19, 2020
@JuanSeBestia
Copy link
Owner

Yep, I think the same. this MR not would be a Breaking Change, but I like promisification

@Rapsssito
Copy link
Collaborator

Entirely removing the callbacks might break older code (mine included) as it is not backwards compatible. A promisfication is tipically a breaking change.

@eliaslecomte
Copy link
Collaborator Author

I think it makes sense to do this, at the moment sometimes for iOS it's via promise, android via callback. That doesn't make sense? Did not yet find the time to do this though.

@JuanSeBestia
Copy link
Owner

Me too

@eliaslecomte
Copy link
Collaborator Author

I'm in the process of changing all used callbacks to promises on Android. Is there still work on this for iOS?

@eliaslecomte
Copy link
Collaborator Author

I think I am done.

@eliaslecomte eliaslecomte requested a review from Rapsssito March 31, 2020 16:19
@eliaslecomte eliaslecomte self-assigned this Mar 31, 2020
@Rapsssito
Copy link
Collaborator

Rapsssito commented Apr 1, 2020

@eliaslecomte, did you test these changes on iOS?

@eliaslecomte
Copy link
Collaborator Author

No, I think all of them were android only. Could you do testing on iOS? I only bave the iOS enlmulator atm.

@Rapsssito
Copy link
Collaborator

@eliaslecomte, I will test it on iOS ASAP.

README.md Outdated Show resolved Hide resolved
@Rapsssito
Copy link
Collaborator

@eliaslecomte, I have tested the example App on iOS with this branch and everything looks fine.

@eliaslecomte eliaslecomte requested a review from Rapsssito April 25, 2020 17:48
@eliaslecomte
Copy link
Collaborator Author

This is ready to be merged 👍.

@Rapsssito
Copy link
Collaborator

LGTM!

@eliaslecomte eliaslecomte merged commit 83e30cd into JuanSeBestia:master May 1, 2020
JuanSeBestia pushed a commit that referenced this pull request May 1, 2020
# [4.0.0](v3.1.2...v4.0.0) (2020-05-01)

* Merge pull request #56 from inthepocket/feature/android-use-promises ([83e30cd](83e30cd)), closes [#56](#56)

### Features

* **promise:** change isEnabled from callback to promise. ([5631fe8](5631fe8))
* **promise:** return the current signal strength as a promise ([7ef9a40](7ef9a40))
* **promise:** use a promise instead of callsbacks for isRemoveWifiNetwork ([abb3be2](abb3be2))
* **promise:** use a promise to resolve the frequency of the currently connected WiFi network ([79dc3bf](79dc3bf))
* **promise:** use a promise to return results for reScanAndLoadWifiList() ([dcdeb0c](dcdeb0c))
* **promise:** use a promise to return the bssid of the currenlty connected network ([8d39c67](8d39c67))
* **promise:** use a promise to return the loadWifiList results ([e0fe2b9](e0fe2b9))

### BREAKING CHANGES

* Use promises instead of callbacks
@JuanSeBestia
Copy link
Owner

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eliaslecomte eliaslecomte deleted the feature/android-use-promises branch June 7, 2020 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants