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

fix(windows, crash): try/catch fetching network profile, new windows example app #511

Merged
merged 10 commits into from Nov 4, 2021

Conversation

tero-paananen
Copy link
Contributor

@tero-paananen tero-paananen commented Oct 29, 2021

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Wow - this is great, thank you for taking the time to update the windows port while fixing this issue.

I have no problem with this approach, and after some more review I can likely approve - I'm just curious if you've seen this and ever attempted it? https://github.com/microsoft/react-native-test-app/

It just showed up on my radar a week or two ago and I haven't had the time to test it, but it appears like it might automate this whole process and take away all manual labor in the future for multi-platform maintenance which would be an absolutely incredible win if reality matched theory

@tero-paananen
Copy link
Contributor Author

I have ever noticed https://github.com/microsoft/react-native-test-app/ i will check that app later

@namrog84
Copy link
Contributor

namrog84 commented Nov 4, 2021

Hey thanks for mentioning that project. First time I am hearing about it. I am curious about it and will check it out. I'd love to have better all app sample project!

@namrog84
Copy link
Contributor

namrog84 commented Nov 4, 2021

Since the temporary key was regenerated, and there is no easy way to verify this thru code PR.
Just confirming it was re-generated with the <PackageCertificatePassword>password</PackageCertificatePassword>

Otherwise all looks great to me! 👍

@mikehardy
Copy link
Contributor

I played with react-native-test-app yesterday as part of re-vamping the example app in https://github.com/invertase/react-native-apple-authentication which had gone out of date.

I'm not sure if I'm doing something wrong or not but I had a 10GB node_modules folder and everything was sloooooowwww

So for me I am very very curious about that project and intend to follow it (and help it if possible) but it's still "in the lab" testing right now, I can't say my experience qualified it for use just yet until I know more.

Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This LGTM - is there any chance this is a breaking change for windows users?
I have no problem with breaking changes if it is one, I just want to make 100% that if it is breaking I do a semver major and we document exactly what steps module users should take to forward-port if so

@mikehardy mikehardy changed the title New Example app (rn-win 0.65.6) for Windows and issue #454 fixed fix(windows, crash): try/catch fetching network profile, new windows example app Nov 4, 2021
@mikehardy mikehardy merged commit ef3ac76 into react-native-netinfo:master Nov 4, 2021
github-actions bot pushed a commit that referenced this pull request Nov 4, 2021
## [6.0.6](v6.0.5...v6.0.6) (2021-11-04)

### Bug Fixes

* **windows, crash:** try/catch fetching network profile, new windows example app ([#511](#511)) ([ef3ac76](ef3ac76)), closes [#454](#454)
@matt-oakes
Copy link
Collaborator

🎉 This PR is included in version 6.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Windows with device's SIM card fails
4 participants