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.03k stars 2.54k forks source link

[No QA][TS migration] Remove all remaining prop-types from the project #42249

Closed blazejkustra closed 3 days ago

blazejkustra commented 2 weeks ago

Details

Remove all remaining prop-types from the project, and uninstall prop-types package!

Fixed Issues

$ https://github.com/Expensify/App/issues/31919 PROPOSAL: N/A

Tests

N/A

Offline tests

N/A

QA Steps

N/A

PR Author Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
melvin-bot[bot] commented 2 weeks ago

@hoangzinh Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

blazejkustra commented 2 weeks ago

I'm unsure if we need a C+ review on this one @hoangzinh, let's wait for internal approval

dubielzyk-expensify commented 2 weeks ago

I assume no design review is needed here if there's no visual changes?

blazejkustra commented 2 weeks ago

@dubielzyk-expensify That's right, no visual changes.

blazejkustra commented 2 weeks ago

FYI TS is failing due to this

hayata-suenaga commented 2 weeks ago

@blazejkustra do we need this to be merged before?

hayata-suenaga commented 2 weeks ago

Because this is just removing prop type related codes, the only thing we should make sure is that there is no TS errors and app builds without an issue. No need for C+ review @hannojg πŸ™‡

I requested an ad hoc build. It might fail because of the TS error (which will be fixed in another PR)

github-actions[bot] commented 2 weeks ago
:test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! :test_tube::test_tube: Android :robot: iOS :apple:
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/42249/index.html https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/42249/index.html
Android iOS
Desktop :computer: Web :spider_web:
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/42249/NewExpensify.dmg https://42249.pr-testing.expensify.com
Desktop Web

:eyes: View the workflow run that generated this build :eyes:

blazejkustra commented 2 weeks ago

@blazejkustra do we need this to be merged before?

@hayata-suenaga Yes, because TS on main was failing, now should be good πŸ‘

hayata-suenaga commented 1 week ago

The PR looks good to me. Because we have a company wide merge freeze right now, I'll put a HOLD on this issue.

blazejkustra commented 5 days ago

I updated the branch, so back to you @hayata-suenaga πŸ˜„

hayata-suenaga commented 5 days ago

I'll put HOLD back to this issue as we're still gradually lifting Merge Freeze and this PR touches several files (although I know this is just removing unused pop types) πŸ™‡

blazejkustra commented 3 days ago

Makes sense! Let's come back to this next week/later πŸ˜„

hayata-suenaga commented 3 days ago

actually, the complete merge freeze lift was announced yesterday. We can merge this now πŸŽ‰

hayata-suenaga commented 3 days ago

Reviewer Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
OSBotify commented 3 days ago

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify commented 2 days ago

πŸš€ Deployed to staging by https://github.com/hayata-suenaga in version: 1.4.77-11 πŸš€

platform result
πŸ€– android πŸ€– failure ❌
πŸ–₯ desktop πŸ–₯ success βœ…
🍎 iOS 🍎 success βœ…
πŸ•Έ web πŸ•Έ success βœ…
OSBotify commented 2 days ago

πŸš€ Deployed to staging by https://github.com/hayata-suenaga in version: 1.4.77-11 πŸš€

platform result
πŸ€– android πŸ€– success βœ…
πŸ–₯ desktop πŸ–₯ success βœ…
🍎 iOS 🍎 success βœ…
πŸ•Έ web πŸ•Έ success βœ