Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.87k forks source link

[HOLD for payment 2024-09-18][$125] Upgrade mapbox and remove patch #48123

Closed roryabraham closed 1 month ago

roryabraham commented 2 months ago

This issue is just a tracker to upgrade mapbox and remove the patch we added in https://github.com/Expensify/App/pull/45289/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R102

Looks like the build issues are being address upstream in https://github.com/rnmapbox/maps/issues/3593 and we can upgrade to https://github.com/rnmapbox/maps/releases/tag/v10.1.30 and remove the patch.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ed1b0e1397924f45
  • Upwork Job ID: 1828535807455065976
  • Last Price Increase: 2024-08-27
  • Automatic offers:
    • suneox | Reviewer | 103729434
    • abzokhattab | Contributor | 103729436
melvin-bot[bot] commented 2 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01ed1b0e1397924f45

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @suneox (External)

melvin-bot[bot] commented 2 months ago

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 months ago

Upwork job price has been updated to $100

melvin-bot[bot] commented 2 months ago

Upwork job price has been updated to $125

Nodebrute commented 2 months ago

@roryabraham I can work on this issue.

roryabraham commented 2 months ago

For the sake of this proposal, let's lay out the test steps we'll follow to ensure that the new version works as expected

abzokhattab commented 2 months ago

Edited by proposal-police: This proposal was edited at 2024-08-27 22:21:47 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Upgrade mapbox and remove patch

What is the root cause of that problem?

The root cause is a compatibility issue between React Native 0.75 and the @rnmapbox/maps package version 10.1.26. A patch was initially applied to address these issues, but the underlying problems are being addressed upstream in rnmapbox/maps by the maintainers so we can remove this patch and upgrade the package instead .

What changes do you think we should make in order to solve the problem?

Upgrade the @rnmapbox/maps package to version 10.1.30, which includes fixes for the React Native 0.75 compatibility issues, and remove the old patch previously applied to this package.

What alternative solutions did you explore? (Optional)

N.A.

suneox commented 2 months ago

@abzokhattab proposal is straightforward to handle the update, so we can go ahead with it.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 2 months ago

Current assignee @roryabraham is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 2 months ago

📣 @suneox 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 2 months ago

📣 @abzokhattab 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

melvin-bot[bot] commented 2 months ago

@suneox, @lschurr, @roryabraham, @abzokhattab Eep! 4 days overdue now. Issues have feelings too...

suneox commented 2 months ago

@abzokhattab Could you please provide an ETA for PR?

abzokhattab commented 2 months ago

working on the PR here https://github.com/Expensify/App/pull/48436

melvin-bot[bot] commented 1 month ago

@suneox, @lschurr, @roryabraham, @abzokhattab Whoops! This issue is 2 days overdue. Let's get this updated quick!

suneox commented 1 month ago

Still waiting @abzokhattab change PR status

abzokhattab commented 1 month ago

Thanks for your patience i was having a problem upgrading Ruby from v2 to v4 .. the PR is now ready

MrRefactor commented 1 month ago

Could we also bump pod for rnmapbox/maps? CC: @abzokhattab @roryabraham

roryabraham commented 1 month ago

good catch @MrRefactor

roryabraham commented 1 month ago

Created https://github.com/Expensify/App/pull/48925 to clean that up

roryabraham commented 1 month ago

Sorry for the difficulty with automations not working in this case. Growing pains in the deploy systems.

The PR for this was deployed to prod on 2024-09-11

lschurr commented 1 month ago

Which PR fixed the bug on this GH @roryabraham?

It looks like this one wasn't deployed to prod yet: https://github.com/Expensify/App/pull/48436

roryabraham commented 1 month ago

https://github.com/Expensify/App/pull/48436. It was deployed to prod on 2024-09-11, the deploy comment just didn't work. Sorry for the confusion

lschurr commented 1 month ago

Payment summary: