metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard

chore: remove unnecessary patch imports and change the patch branch name

Open OGPoyraz opened this issue 1 year ago • 4 comments

Description

This PR removes unnecessary files from corrupted patch introduced in the https://github.com/MetaMask/metamask-mobile/pull/9442/files#diff-83d0f6fc92e49e38dc038764255880631a481838b47b7dbaf40837264502d5e2

The reason that these lines/files added in that PR is not having a clean build in the core right before patching.

For example if you build core in the main branch right now, you will have some files under helpers e.g transaction-controller/dist/helpers/EtherscanRemoteTransactionSource.d.ts. But in the patch branch these files are under the root folder transaction-controller/dist/EtherscanRemoteTransactionSource.d.ts. So if you don't clean your local before patching, these files will be also included. To prevent this issue, do a yarn build:clean in the core package before patching in the mobile.

We also keep the branch name as is and expect developers open PR against it, if it gets merged in to main mobile, then we update the patch branch.

Patch branch: https://github.com/MetaMask/core/compare/main...patch/mobile-transaction-controller-13-0-0

Related issues

N/A

Manual testing steps

N/A

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • [x] I’ve followed MetaMask Coding Standards.
  • [x] I've completed the PR template to the best of my ability
  • [ ] I’ve included tests if applicable
  • [ ] I’ve documented my code using JSDoc format if applicable
  • [x] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

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.

OGPoyraz avatar May 22 '24 09:05 OGPoyraz

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: d39461f3fed61a90959153f73dd80f68efa66c11 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9001c285-49aa-4421-b05a-431226b1749a

[!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

github-actions[bot] avatar May 22 '24 09:05 github-actions[bot]

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarqubecloud[bot] avatar May 22 '24 10:05 sonarqubecloud[bot]

Amazing cleanup @OGPoyraz This looks awesome!

Just add manual steps testing for the release qa, a video showing a transactions and dapp transaction working, and the core branch that it was used to generate the patch and we are good to merge

Requested a regression from @sleepytanya

OGPoyraz avatar May 22 '24 12:05 OGPoyraz

Nice work @OGPoyraz!

You've captured important document in the PR description. Should we start a PATCHING_CORE_PACKAGES.md to document the correct steps for patching? Is there a way to add the yarn build:clean step (for the core repo) to patch-transaction-controller.sh?

dbrans avatar May 22 '24 12:05 dbrans

QA build Bitrise

  • [x]  Transactions - send native token origin MM
  • [x]  Transactions - send native token origin dapp
  • [x]  Transactions - send ERC20 token origin MM
  • [x]  Transactions - send ERC20 token origin dapp
  • [x]  Transactions - send ERC721 token origin MM
  • [x]  Transactions - send ERC721 token origin dapp
  • [x]  Transactions - speed up transaction
  • [x]  Transactions - cancel transaction
  • [x]  Speed up cancellation
  • [x]  Tokens - import ERC20 token origin MM
  • [x]  Tokens - import ERC20 token origin dapp
  • [x]  Tokens - import ERC721 token origin MM
  • [x]  Tokens - import ERC721 token origin dapp
  • [x]  Tokens - import ERC1155 token origin MM
  • [x]  Tokens - import ERC1155 token origin dapp
  • [x]  Tokens - ERC1155 transfer origin MM
  • [x]  Tokens - ERC1155 batch transfer
  • [x]  Tokens - approve ERC1155 token
  • [x]  Tokens - approve ERC20 token
  • [x]  Tokens - approve ERC721 token
  • [x]  ENS - name resolution
  • [x]  Gas fee - EIP-1559 gas
  • [x]  Gas fee - legacy gas
  • [x]  Sign - ETH sign
  • [x]  Sign - personal sign
  • [x]  Sign - sign in with Ethereum
  • [x]  Sign - sign typed with data (v3 and v4)
  • [x]  Swap - swap tokens
  • [x]  Incoming transactions history
  • [x]  Custom nonce
  • [x]  Cancel transaction using custom nonce
  • [x]  Stuck multiple transactions (cancel / speed up)
  • [x]  PPOM

sleepytanya avatar May 23 '24 05:05 sleepytanya