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

[HOLD for payment 2023-04-20] [$4000] Update RN version to our latest v0.71.2-alpha.3 #15124

Closed
mountiny opened this issue Feb 14, 2023 · 62 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@mountiny
Copy link
Contributor

mountiny commented Feb 14, 2023

Problem

To fix some bugs without patches we need to update our RN fork to newer version, we had merged those changes to our fork https://github.com/Expensify/react-native/releases/tag/v0.71.2-alpha.1 and we would like to include that in the App

Why

Includes some new fixes in the react-native fork

Solution

Update App's RN version to our latest https://github.com/Expensify/react-native/releases/tag/v0.71.2-alpha.1

cc @roryabraham

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0151c999f86eec33a8
  • Upwork Job ID: 1625450113750552576
  • Last Price Increase: 2023-04-13
@mountiny mountiny added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 14, 2023
@MelvinBot
Copy link

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot changed the title Update RN version to our latest v0.71.2-alpha.1 [$1000] Update RN version to our latest v0.71.2-alpha.1 Feb 14, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~0151c999f86eec33a8

@MelvinBot
Copy link

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@MelvinBot
Copy link

Triggered auto assignment to @laurenreidexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 14, 2023
@MelvinBot
Copy link

Triggered auto assignment to @chiragsalian (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@mountiny
Copy link
Contributor Author

Unassigning Lauren as that was an autoassigner blip.

here is a draft PR inspiration #14618

@priyeshshah11
Copy link
Contributor

priyeshshah11 commented Feb 14, 2023

@mountiny How is the proposal process going to work for this? I would like to do it if it's going to be on a volunteer basis, because there is not much to propose here

Proposal

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

We are trying to upgrade the app's react native version as it solves some of the bugs.

What is the root cause of that problem?

We are using older version of our react native fork.

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

Upgrade the react native dependency version to v0.71.2-alpha.1 as stated in the issue.

@abdulrahuman5196
Copy link
Contributor

@mountiny Is this open for external contribution? If so I would volunteer to contribute?

@mountiny
Copy link
Contributor Author

This is going to be involved and take lots of testing and ideally someone already has experience with updating the RN version. I was thinking of offering this to C+ because of the importance and non-triviality of this change.

@priyeshshah11 @abdulrahuman5196 Would you both want to collaborate on this? One can make the PR and the other one will be testing extensively and suggesting any improvements if need be?

In terms of the reward, if all goes well and we can merge this soon we got $1500 split between you two? Would that work?

for better collaboration you could discuss anything in Slack thread to speed up the progress. thoughts?

@mountiny
Copy link
Contributor Author

Here is a WIP branch which can be inspiration #14618

And here is a helper tool which you can use to know what to update also https://react-native-community.github.io/upgrade-helper/

@priyeshshah11
Copy link
Contributor

I would be happy to collaborate & create the PR.

@abdulrahuman5196
Copy link
Contributor

@mountiny @priyeshshah11 Yes. I would be happy to collaborate and work. We can sync up over slack thread to discuss on tasks to move forward?

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 14, 2023
@MelvinBot
Copy link

📣 @priyeshshah11 You have been assigned to this job by @mountiny!
Please apply to this job in Upwork 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 📖

@MelvinBot
Copy link

📣 @aimane-chnaif You have been assigned to this job by @mountiny!
Please apply to this job in Upwork 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 📖

@mountiny
Copy link
Contributor Author

No regression tests required, this was not regression from nothing as well.

@abdulrahuman5196 @priyeshshah11 Are you happy to split the reward as $1000 to Abdul and $2000 to Priyesh? Its been a lot of work but I feel like Priyesh had a bit more work to do here, feel free to propose otherwise.

There will be also $1000 to @aimane-chnaif and $1000 @Santhosh-Sellavel as C+ review and testing, does this work?

@abdulrahuman5196
Copy link
Contributor

@mountiny I too agree. @priyeshshah11 did good amount of work. I am fine with the split up.

@priyeshshah11
Copy link
Contributor

Really appreciate that @mountiny & @abdulrahuman5196 ❤️ I am happy with that decision but just saying that it took a lot more time & effort than initially imagined thus I think a bit more than $2000 would be a fair payout but I am happy with whatever you & the team think is fair 👍

@mountiny
Copy link
Contributor Author

@priyeshshah11 @abdulrahuman5196 I agree it was a lot of work. I am happy to do $1250 and $2750 split of $4K total on this one.

@mountiny mountiny changed the title [HOLD for payment 2023-04-20] [$3000] Update RN version to our latest v0.71.2-alpha.3 [HOLD for payment 2023-04-20] [$4000] Update RN version to our latest v0.71.2-alpha.3 Apr 13, 2023
@MelvinBot
Copy link

Upwork job price has been updated to $4000

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 15, 2023

@priyeshshah11 If I am pointing correctly 2 issues of icons are raised after this PR change where in android Text -> View -> svg are not rendering properly.

#17368
#17433

@priyeshshah11
Copy link
Contributor

@priyeshshah11 If I am pointing correctly 2 issues of icons are raised after this PR change where in android Text -> View -> svg are not rendering properly.

#17368 #17433

@Pujan92 I have been looking into these issues for past couple of days & I don't know what's exactly wrong nor I have a solution yet but I don't think it's caused by this PR. I also tried upgrading react-native-svg to v13.9.0 & svgr/webpack to v7.0.0 but no luck. It's weird as I can see the icons appear as soon as I save the files but they still appear in the wrong place. @mountiny maybe someone from the software mansion team can help here?

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 18, 2023

@priyeshshah11 Can this connect to the version of react-native? bcoz before posting here I switched to the earlier commit of the old version and reinstalled it with that and it seemed to work.

@aimane-chnaif
Copy link
Contributor

It would be good to have minimum RN project with Text -> View -> svg on different versions and verify that it's really RN version issue.

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 18, 2023

I just tried upgrading the version of react-native to 0.71.6 and it showing the icons, maybe you can just give it a try once to confirm

Screenshot 2023-04-18 at 6 10 16 PM

@aimane-chnaif
Copy link
Contributor

ok, then there should be GH in RN repo which fixed this (between 0.71.2-alpha.3 and 0.71.6).
Let's do further discussions in this open issue.

@Pujan92
Copy link
Contributor

Pujan92 commented Apr 18, 2023

To update, it works for RN version 0.71.2 also. I doubt that something fishy with the forked repo in our app of version 0.71.2-alpha.3. I believe software mansion team can look and help it over here as I am seeing some manipulation happened in android text layout files.

@mountiny
Copy link
Contributor Author

Feel free to bring those issues to Slack @Pujan92 we can tag people who can help with it then, that will be easier

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 19, 2023
@maddylewis
Copy link
Contributor

Let me know if the following is accurate before I process payment - thanks!

payments:

@maddylewis
Copy link
Contributor

offers sent

@mountiny
Copy link
Contributor Author

looking good, thanks everyone!

@priyeshshah11
Copy link
Contributor

@maddylewis I got the offer but I can't accept it as it shows as withdrawn, could you please resend?

@abdulrahuman5196
Copy link
Contributor

@maddylewis same here

@priyeshshah11

This comment was marked as off-topic.

@priyeshshah11
Copy link
Contributor

accepted, thanks! @maddylewis

@melvin-bot melvin-bot bot added the Overdue label Apr 24, 2023
@melvin-bot melvin-bot bot added Reviewing Has a PR in review and removed Overdue labels Apr 24, 2023
@abdulrahuman5196
Copy link
Contributor

@maddylewis Seems this issue is closed. I have accepted the upwork request, but haven't got paid out.

@maddylewis
Copy link
Contributor

@abdulrahuman5196 - just paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests