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

[New Arch] Fabric Example #3175

Merged

Conversation

yungblud
Copy link
Contributor

@yungblud yungblud commented Jul 19, 2023

Thanks for opening a PR!
Since this is a volunteer project and is very active, anything you can do to reduce the amount of time needed to review and merge your PR is appreciated.
The following steps will help get your PR merged quickly:

Update the documentation

If you've added new functionality, update the README.md with an entry for your prop or event.
The entry should be inserted in alphabetical order.

Update the changelog

After you open the PR, update the CHANGELOG.md file with an entry pointing to your PR.

Provide an example of how to test the change

cd examples/FabricExample
yarn pod-install
yarn ios or yarn android

Focus the PR on only one area

Testing multiple features takes longer than isolated changes and if there is a bug in one feature, prevents the other parts of your PR from getting merged until it gets fixed.
If you're touching multiple different areas that aren't related, break the changes up into multiple PRs.

Describe the changes

We are splitting commits by features from #3122
This includes only for creating FabricExample app. (nothing else were implemented about Fabric)
Android build would be crashed on the build time.
This wouldn't be a big issue. (I think this problem is occurred because we didn't implement Fabric on react-native-video but we are trying to build with Fabric on FabricExample)

@yungblud yungblud marked this pull request as ready for review July 19, 2023 08:00
@freeboub
Copy link
Collaborator

freeboub commented Aug 3, 2023

Looks good, thank you !
I just release a new alpha version, we can merge this PR now !
And sorry for the delayed answer

@freeboub freeboub merged commit 30d8146 into TheWidlarzGroup:master Aug 3, 2023
1 check passed
@yungblud
Copy link
Contributor Author

yungblud commented Aug 7, 2023

Looks good, thank you ! I just release a new alpha version, we can merge this PR now ! And sorry for the delayed answer

Thanks!
I will create another PR.
The next PR will be Android Fabric implement. (I guess)

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.

3 participants