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.

Sign up for GitHub

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

Jump to bottom

feat: Added DeviceInfo.supportedABIs() #598

Merged
merged 9 commits into from
Mar 21, 2019
Merged

feat: Added DeviceInfo.supportedABIs() #598

merged 9 commits into from
Mar 21, 2019

Conversation

hemantasapkota
Copy link
Contributor

@hemantasapkota hemantasapkota commented Mar 21, 2019 •
edited
Loading

Description

Fixed issue #578

Added supportedABIs() which returns a list of supported device cpu types

Compatibility

OS Implemented
iOS ✅
Android ✅
Windows ❌

Checklist

  • I have tested this on a device/simulator for each compatible OS
  • I added the documentation in README.md.
  • I mentioned this change in CHANGELOG.md.

machour
machour reviewed Mar 21, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
@hemantasapkota hemantasapkota marked this pull request as ready for review March 21, 2019 09:25
@machour machour requested a review from mikehardy March 21, 2019 10:26
mikehardy
mikehardy requested changes Mar 21, 2019
View reviewed changes
Copy link
Collaborator

@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.

One change to make sure there is an entry on Android for API<LOLLIPOP

android/src/main/java/com/learnium/RNDeviceInfo/RNDeviceModule.java Show resolved Hide resolved
@mikehardy
Copy link
Collaborator

mikehardy commented Mar 21, 2019 •
edited
Loading

@hemantasapkota this looks great, I'm excited to have this feature in the module. @machour I looked closely and I can't see anything else I'd change in here? I'm not perfect though of course so I'll leave the merge for you in case you want to take another look

@hemantasapkota
Copy link
Contributor Author

@mikehardy thanks for the review. This has been a good experience. looking forward to @machour 's review.

machour
machour approved these changes Mar 21, 2019
View reviewed changes
Copy link
Contributor

@machour machour left a comment

Choose a reason for hiding this comment

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

Thank you so much for this contribution @hemantasapkota, it will be shipped later today 🎉

@mikehardy mikehardy changed the title Added DeviceInfo.supportedABIs() feat: Added DeviceInfo.supportedABIs() Mar 21, 2019
@mikehardy mikehardy merged commit 5f121cb into react-native-device-info:master Mar 21, 2019
@mikehardy
Copy link
Collaborator

Whether I got that merge correct or not 🙏 - it is my first merge into this repo. Thanks @hemantasapkota for the work and @machour for the help

@machour
Copy link
Contributor

machour commented Mar 21, 2019

👌

react-native-device-info 1.1.0 published 🎉

@mikehardy
Copy link
Collaborator

I can see in the commit log the way I did it duplicated the message (the title was the same as the commit message) which isn't as tidy as your previous ones. I'll make sure the title is complete/correct next time, but leave the message empty and they'll match up. Not an A+ yet ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@machour machour machour approved these changes

@mikehardy mikehardy mikehardy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hemantasapkota @mikehardy @machour