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

docs(index.md): Expo / EAS updates and fixes #7502

Merged
merged 19 commits into from
Jan 23, 2024

Conversation

gregfenton
Copy link
Contributor

Fixed a few formatting issues & divided the section into subsections for clarity in following the steps.

Description

Motivation: people (mainly newcomers to Expo) in the Firebase community struggling to understand the guidance for Expo.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan


Think react-native-firebase is great? Please consider supporting the project with any of the below:

Copy link

vercel bot commented Dec 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 23, 2024 3:49pm
react-native-firebase-next ❌ Failed (Inspect) Jan 23, 2024 3:49pm

@luccasr73
Copy link

luccasr73 commented Dec 13, 2023

@gregfenton "useFrameworks": "static" no longer mandatory?

@gregfenton
Copy link
Contributor Author

@gregfenton "useFrameworks": "static" no longer mandatory?

I think it is still required? I kept the information about it, but moved it around a bit. The setting is in the JSON example and the paragraph about it is moved to just before the JSON example.

An issue I saw when useFramework was in a separate part of the docs from the Config Plugins is that people were missing it. So merging them means that someone doing a rapid scan of the docs should get everything they need from the one JSON snippet, not two.

@mikehardy
Copy link
Collaborator

The way it is done in Expo may change, but the underlying requirement for use_frameworks! :linkage => :static to be in the Podfile configuration (somehow) is still an existing strict requirement for builds to work

@gregfenton
Copy link
Contributor Author

I kept the required info, just moved it around for easier consumption by "fly-by" readers 😁

Copy link
Contributor

@amandeepmittal amandeepmittal left a comment

Choose a reason for hiding this comment

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

I was going to open a PR but saw this. Hope you don't mind me intervening your PR @gregfenton and hoping Mike doesn't me leaving review too (🙈 )

I've left some suggestions based on recent Expo docs changes and also how things might be going on moving forward so as to eliminate possibilities for creating confusion.

Also, I couldn't edit or suggest the introduction paragraph under Expo so can I ask you to please update it to this:

Integration with Expo is possible with both with [development builds](https://docs.expo.dev/workflow/overview/#development-builds) or [bare workflow](https://docs.expo.dev/bare/overview/).

I think instead of calling them custom managed workflows/managed workflows, it is more appropriate now to call them just development builds.

Thank you so much, Greg, for updating this 🙏

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
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.

Thanks for this! Will try to shepherd this through all the CI checks right now
I made one fix as the use frameworks thing was in android instead of ios - I don't think that's controversial at all so I'm going to just commit it and get this in 🚂

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
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.

4 participants