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

checkInternetConnectionOnStartup to check internet connection not only with NetInfo #94

Merged
merged 10 commits into from
May 30, 2018
Merged

checkInternetConnectionOnStartup to check internet connection not only with NetInfo #94

merged 10 commits into from
May 30, 2018

Conversation

dickerpulli
Copy link
Contributor

No description provided.

Copy link
Owner

@rgommezz rgommezz left a comment

Choose a reason for hiding this comment

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

Just some flow things related. Other than that, LGTM to be merged

): Promise<boolean> {
let connectionChecked: Promise<boolean>;
if (Platform.OS === 'ios') {
connectionChecked = new Promise(resolve: Function => {
Copy link
Owner

@rgommezz rgommezz May 30, 2018

Choose a reason for hiding this comment

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

There is a flow parsing error on the CI, could you please wrap (resolve: Function) into parenthesis?

I don't know if that's due to an old version (v0.42). Another alternative could be to upgrade flow to a most recent one.

But probably the easiest is the former

connectionChecked = NetInfo.isConnected.fetch();
}

return connectionChecked.then(isConnected: boolean => {
Copy link
Owner

Choose a reason for hiding this comment

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

Same thing here

Copy link
Owner

@rgommezz rgommezz left a comment

Choose a reason for hiding this comment

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

Thanks very much for this @dickerpulli!

@rgommezz rgommezz merged commit a8deeb7 into rgommezz:master May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants