-
Notifications
You must be signed in to change notification settings - Fork 787
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
improvement way to get root SDK versions #129
Conversation
just added a function which acts like a ternary with fallback option. Hence, less cluttered "def" variables additionally, changed the SDK values from 23 to 26 as per new changes from react-native and Android Android Target API Level 26 will be required in August 2018. https://android-developers.googleblog.com/2017/12/improving-app-security-and-performance.html And the React Native team is already working on this: facebook/react-native#18095 facebook/react-native#17741 PS: I am aware of this PR joltup#128 but first its still targeting old SDK values i.e 23 as fallback and secondly I am not sure of the use of `project` to get the value instead of proper way i.e `rootProject.ext` to get the property value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 40 syntax error
android/build.gradle
Outdated
@@ -33,7 +37,7 @@ android { | |||
} | |||
|
|||
dependencies { | |||
compile 'com.facebook.react:react-native:+' | |||
compile 'com.facebook.react:react-native:${safeExtGet('reactNativeVersion', '+')}' | |||
//compile 'com.squareup.okhttp3:okhttp:+' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yeomann Thanks for this commit--it got me past the first hurdle adding rn-fetch-blob to a new project. Line 40 threw an exception during build though until I wrapped the string in double quotes. Could you apply a fix like that for the single quotes in single quotes issue?
(I haven't officially been added to this project yet, but I'm working with @Traviskn to merge this request as soon as possible.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh balls! my bad, it was actually suppose to be wrapped with double quotes, I have tested this multiple times, single quotes within single quotes doesn't work. let me fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed line 40, wrapped in double quotes
Thanks! |
…e64-dependency added missing base64 dependency
just added a function which acts like a ternary with fallback option. Hence, less cluttered "def" variables
additionally, changed the SDK values from 23 to 26 as per new changes from react-native and Android
Android Target API Level 26 will be required in August 2018.
https://android-developers.googleblog.com/2017/12/improving-app-security-and-performance.html
And the React Native team is already working on this:
facebook/react-native#18095
facebook/react-native#17741
PS: I am aware of this PR #128 but first its still targeting old SDK values i.e 23 as fallback and secondly I am not sure of the use of
project
to get the value instead of proper way i.erootProject.ext
to get the property value.