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

convert to new react native config format #408

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Conversation

Traviskn
Copy link

No description provided.

@Traviskn
Copy link
Author

according to the react-native cli docs: hooks are considered anti-pattern, please avoid them
I'm not sure that we should continue to support the prelink script that auto-adds the android manifest permissions. it would likely be best to just document the required permissions and require that you add them yourself

@IntelliJAbhishek
Copy link

@Traviskn Just add required permission in docs. No need to autolink permission. A proper docs would be enough.

@oleksandr-dziuban
Copy link

@IntelliJAbhishek Hello, can we merge this PR?

@IntelliJAbhishek
Copy link

IntelliJAbhishek commented Aug 12, 2019

@oleksandr-dziuban maintainers of this repo can merge this PR.

@filmhomage
Copy link

filmhomage commented Aug 12, 2019

+1 Same here. Please merge this PR.

@congnguyen91
Copy link

+1 please check this PR & merge :) thanks

2 similar comments
@caigehui
Copy link

+1 please check this PR & merge :) thanks

@caigehui
Copy link

+1 please check this PR & merge :) thanks

@BaderSerhan
Copy link

Waiting for this merge ! :)

Copy link

@mvanroon mvanroon left a comment

Choose a reason for hiding this comment

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

I don't think we need the react-native.config.js script at all.
The script installs permissions, but these are already mentioned in the readme

@romreed
Copy link

romreed commented Sep 6, 2019

When this merge will be in master?

@mvanroon
Copy link

mvanroon commented Sep 6, 2019

When this merge will be in master?

When someone merges the PR?

@oleksandr-dziuban
Copy link

oleksandr-dziuban commented Sep 10, 2019

@Traviskn Hello, is it possible to merge this PR? Or this library is not maintained anymore?

@oleksandr-dziuban
Copy link

I don't think we need the react-native.config.js script at all.
The script installs permissions, but these are already mentioned in the readme

We need it anyway to mograte to latest react-native standards, so could you please merge it?

@Traviskn
Copy link
Author

merging for now, planning on a follow up PR to update the README and remove the hook entirely

@Traviskn Traviskn merged commit 356d731 into master Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants