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: Remove react-native-flipper to prevent build issues when running yarn start:ios #11725

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Cal-L
Copy link
Contributor

@Cal-L Cal-L commented Oct 9, 2024

Description

Devs have been reporting weird build issues on iOS, which are related to react-native-flipper being added a while back. This package is used by the redux debugger plugin in Flipper so the removal should have minimal impact especially since the usage is low. We will look into adding this back in the future. For now, we will remove this package to unblock contributors from building iOS. NOTE: This does not prevent Flipper from working.

Related issues

Fixes:

Manual testing steps

  1. yarn setup
    2.yarn start:ios
    3.App should build on this branch

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@Cal-L Cal-L requested review from a team as code owners October 9, 2024 23:02
@Cal-L Cal-L added the No QA Needed Apply this label when your PR does not need any QA effort. label Oct 9, 2024
Copy link
Contributor

github-actions bot commented Oct 9, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@Cal-L Cal-L added Run Smoke E2E Triggers smoke e2e on Bitrise needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-mobile-platform and removed team-mobile-platform labels Oct 9, 2024
Copy link
Contributor

github-actions bot commented Oct 9, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 7b9b190
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/096f2e44-f6a9-47e8-a6dd-460c69c85427

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/[email protected]

View full report↗︎

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.21%. Comparing base (5b1d54a) to head (7b9b190).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11725      +/-   ##
==========================================
+ Coverage   54.07%   54.21%   +0.14%     
==========================================
  Files        1689     1709      +20     
  Lines       38546    38708     +162     
  Branches     4707     4736      +29     
==========================================
+ Hits        20844    20987     +143     
- Misses      16264    16272       +8     
- Partials     1438     1449      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Oct 9, 2024

Copy link
Contributor

@kylanhurt kylanhurt left a comment

Choose a reason for hiding this comment

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

LGTM but broke the e2e smoke test

@Cal-L
Copy link
Contributor Author

Cal-L commented Oct 10, 2024

LGTM but broke the e2e smoke test

Hm that's no good. Let's not merge this yet.

@Cal-L Cal-L added the DO-NOT-MERGE Pull requests that should not be merged label Oct 10, 2024
@Cal-L Cal-L marked this pull request as draft October 10, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE Pull requests that should not be merged needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) No QA Needed Apply this label when your PR does not need any QA effort. Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform
Projects
Status: Needs more work from the author
Development

Successfully merging this pull request may close these issues.

3 participants