chore(router-dom): upgrade react-router-dom

developerfred commented 2 years ago

Description

Upgrade React Router

close #675

Ref

Notice

Before submitting a pull request, please make sure you have answered the following:

[x] Have you followed the guidelines in our Contributing guide?
[x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
[] Do all new and existing tests pass? Does the linter pass?

Pull Request Type

[ ] :bug: Bug fix (Non-breaking Change: Fixes an issue)
[x] :hammer_and_wrench: Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
[ ] :nail_care: New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

If applicable, please link to the github issue and put closes #XXXX in your comment to auto-close the issue that your PR fixes.

Testing

Please outline all testing steps

Pull branch locally and run yarn to install new deps
etc...

Screenshots (if applicable)

0xdef1cafe commented 2 years ago

@developerfred thanks so much for this - i know this is only in draft, but you can run yarn lint:fix to autofix lint errors.

let us know if you have any feedback/need assistance testing this - we have an ops team that can run a regression test before merging.

@Neverwas-dev this one might be coming your way for a general regression test

developerfred commented 2 years ago

@developerfred thanks so much for this - i know this is only in draft, but you can run yarn lint:fix to autofix lint errors.

let us know if you have any feedback/need assistance testing this - we have an ops team that can run a regression test before merging.

@Neverwas-dev this one might be coming your way for a general regression test

I thank you for the opportunity to contribute, for now I'm still running and fixing the lint errors.

if necessary I will notify you.

But thinking aloud I think it would be nice to test Dapp and as a contract interaction function.

developerfred commented 2 years ago

done @0xdef1cafe

0xdef1cafe commented 2 years ago

hey @developerfred - looking good, but unfortunately this is failing type checking in CI

are you able to run yarn type-check locally and fix the type errors locally and push the fixes?

cheers.

developerfred commented 2 years ago

@0xdef1cafe I'll take a look at that now.

0xdef1cafe commented 2 years ago

hey @developerfred - just a heads up, we have a few more pages/routes coming within the next week, so we're going to hold off on merging this until those are done and in production.

we'd still like to get this upgrade done, and we're happy to adjust the bounty for the additional workload involved, but don't feel any need to rush this in within the next few days.

we'll ping you when the additional work is completed and would love to see this get finished then if you're cool with that?

developerfred commented 2 years ago

hey @developerfred - just a heads up, we have a few more pages/routes coming within the next week, so we're going to hold off on merging this until those are done and in production.

we'd still like to get this upgrade done, and we're happy to adjust the bounty for the additional workload involved, but don't feel any need to rush this in within the next few days.

we'll ping you when the additional work is completed and would love to see this get finished then if you're cool with that?

I agree, yes, I'm waiting, I think now I only had 10 warnings :D

I hope thank you and gladly accept the tip.

0xdef1cafe commented 2 years ago

hey @developerfred - develop has stabilized and this one is ready to go again. you're going to have a few conflicts but we can compensate if there's significant additional work, please reach out to @0xean if that's the case 🤘

cjthompson commented 2 years ago

@developerfred - we're ready to have you update this PR against current develop.

developerfred commented 2 years ago

we're ready to have you update this PR against current develop.

Ok, I'll update

0xean commented 2 years ago

@developerfred - please ping us when you are ready for another pass on this one. Thanks again for being flexible with our team!

developerfred commented 2 years ago

@0xean Ok, I can get back to that at the beginning of the week, do you have a preference on another task?

0xean commented 2 years ago

preference for another issue for you to pick up? or I am not sure I am understanding the question.

developerfred commented 2 years ago

preference for another issue for you to pick up? or I am not sure I am understanding the question.

I thought you had closed my pull request, I looked it up quickly so I asked. So I intend to come back to this issue at the beginning of the week. All right for you?

0xean commented 2 years ago

ahh, nope! Would love to get this PR merged once it's ready to go. Sorry for the confusion, whenever you are able to get to it is great.

developerfred commented 2 years ago

ahh, nope! Would love to get this PR merged once it's ready to go. Sorry for the confusion, whenever you are able to get to it is great.

Perfect, thanks for understanding.

0xdef1cafe commented 2 years ago

@developerfred can you please 1. fix the merge conflict marker, and 2. revert all of the unrelated UI layer prop changes if they're not required, or use single quotes please

vercel[bot] commented 2 years ago

@developerfred is attempting to deploy a commit to the ShapeShift Team on Vercel.

A member of the Team first needs to authorize it.

0xean commented 2 years ago

okay, building looks good now, but its failing the yarn type-check will also confirm locally.

0xdef1cafe commented 2 years ago

@developerfred you might want to try yarn local-ci to run essentially the same steps CI does

developerfred commented 2 years ago

done @0xdef1cafe and @0xean

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/shapeshift/web/6Z9P2MhAMUdW3V3LGL2PtdWiqmAw
✅ Preview: Canceled

0xdef1cafe commented 2 years ago

done @0xdef1cafe and @0xean

getting closer! few failing tests in CI and we'll be good to get this regression tested

developerfred commented 2 years ago

done @0xdef1cafe and @0xean

getting closer! few failing tests in CI and we'll be good to get this regression tested

Test has been fixed on d7dc0ef

0xdef1cafe commented 2 years ago

hey @developerfred - given we keep falling out of sync, and this needs a full regression test, we're going to close this PR, reopen it under the shapeshift repo and manage getting it merged internally, and pay you out for the work done on the bounty.

developerfred commented 2 years ago

@0xdef1cafe ok! Thanks!

shapeshift / web