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: vector-icons android double-include, ios navigation stack crash, dependencies #50

Merged

Conversation

mikehardy
Copy link
Collaborator

Each commit here is it's own thing really, but easier to do one PR with multiple clean commits that may be reviewed one at a time easily, then rebase-merge, versus 7 different PRs I think

Each commit has a descriptive message and should be clean, anyway :-)

we include the react-native-vector-icons gradle script in our build.gradle,
that script already takes care of merging in the font files as needed

Including them in a fixed location in android assets is actually a
double-inclusion. This was hidden before because the vector-icons
gradle script moved them to the same spot, but with v9.1.0 of that
package it exposed the issue with a build error.

Delete them here, android still has icons, Fixes LunatiqueCoder#49
Strange that this is necessary, but it avoids a javascript crash
on iOS only, in combination with react-navigation

See invertase/react-native-firebase-authentication-example#9 (comment)
includes result of `./gradlew wrapper`
upstream fixed the issue that made this unreliable, so now it is
once again the most reliable way to install pods
each of these are settings that will improve ios build performance
- turning off warnings is useful because the build is so noisy it affects performance, and it is not actionable since it's not our code
- turning on relative path usage for compiler binaries allows ccache use
@mikehardy mikehardy requested a review from LunatiqueCoder March 5, 2022 21:09
@LunatiqueCoder LunatiqueCoder merged commit 7890c6e into LunatiqueCoder:master Mar 5, 2022
@LunatiqueCoder
Copy link
Owner

Niiice, thank you. I believe this solves #49 ?

@mikehardy
Copy link
Collaborator Author

Yep! And with my Fixes notation in the commit message it auto-closes, I'm all github-optimized ;-)

@mikehardy mikehardy deleted the @mikehardy/dependency-updates branch March 5, 2022 23:19
@LunatiqueCoder
Copy link
Owner

My bad, it's almost 2AM here and my head is on another planet right now. But I will definitely sleep better with these changes 🚀 appreciate it 🙏

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.

2 participants