chore(js-ts): Convert app/util/test/ganache-contract-address-registry.js to TypeScript #11406
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
devin-ai-integration
bot
added
needs-dev-review
…-{{random_number_3_digits}}
# Pull Request Description ## Motivation This pull request aims to convert the `bytes.js` file to TypeScript, improving type safety and maintainability of the codebase. ## Context As part of our ongoing efforts to migrate the MetaMask Mobile codebase to TypeScript, we are converting JavaScript files to TypeScript one at a time. This PR focuses on the `bytes.js` utility file. ## Changes - Renamed `app/util/bytes.js` to `app/util/bytes.ts` - Added type annotations for the function parameter and return value - Refactored the `for` loop to a `for-of` loop to address potential linting issues - Created `notes.md` to track TODOs and type-related considerations ## Files Changed 1. `app/util/bytes.ts` (renamed from `bytes.js`) 2. `notes.md` (new file) ## Testing - Ran `yarn tsc` to ensure successful TypeScript compilation - Ran `yarn lint` to check for any linting issues ## Notes - The `value` parameter type is currently set to `Uint8Array`, but we may need to consider allowing `number[]` as well in the future. - All existing functionality should remain unchanged. ## Checklist - [x] Code follows the MetaMask Coding Standards - [x] Tests for the changes have been added (if applicable) - [x] All tests pass - [x] Lint and prettier checks pass - [ ] This PR is ready for review ## Link to Devin run https://preview.devin.ai/devin/0a3389f83f8d4560bdce6a8a5a186bdd ## Additional Information This pull request was created by Devin at the request of Moritz. If you have any feedback, you can leave comments in the PR and I'll address them in the app! --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR refactor the usage of Logger util as requested in [this](#11250 (comment)) comment. ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Adds logic in the `sonar-cloud-quality-gate-status` step in the CI that checks for the existence of `skip-sonar-cloud` label. If it exists, the step is bypassed and passes. If the label doesn't exist, it checks the sonar cloud quality gate status as usual. Also removes the redundant `sonar-cloud` dependency in the `all-jobs-pass` step ## **Related issues** Fixes: #11403 ## **Manual testing steps** - Apply `skip-sonar-cloud` label - Manually re-run `ci.yml` jobs - Notice the `sonar-cloud-quality-gate-status` step logs that it is skipped and the status checks is green - Afterwards, remove `skip-sonar-cloud` label - Manually re-run `ci.yml` jobs - Notice the `sonar-cloud-quality-gate-status` step runs as usual ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> With label applied https://github.com/MetaMask/metamask-mobile/actions/runs/11026106803/job/30622332596?pr=11425 Without label applied **https://github.com/MetaMask/metamask-mobile/actions/runs/11026106803/job/30623782793** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR adds ANNOUNCEMENT_NOTIFICATION_CHANNEL_ID for AndroidChannels. ## **Related issues** Fixes: [11415](#11415) ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR adds a withTimeout wrapper on notifee async calls to avoid get blocked in case a call doesn't resolve quickly. This was a comment from [this PR](#11250 (comment)) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR fixes a bug introduces on iOS builds using react-native-quick-crypto as documented [here](margelo/react-native-quick-crypto#244) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** Successful build: https://app.bitrise.io/build/076ad837-6e35-4165-a4f2-14c5754ad1d3 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Adds tracking to the user requesting network switching as well as his press of the confirmation button. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3299 ## **Manual testing steps** 1. Go to the in app browser and open have a connected dapp for example to ethereum main net 2. In the dapp select to change network, the modal will come up, thats when the tracking event if fire for user requesting a network change 3. Confirm the network change by pressing the button, this corresponds to the confirm pressed tracked event ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> #### Mixpanel logs for the two events <img width="1024" alt="Mixpanel logs for the two events" src="https://github.com/user-attachments/assets/6ab51bd5-42a3-4564-94e3-6c8f76999cb8"> #### Network Switch Requested and Modal Shown | Interaction | Tracked event | |--------------|--------------| | <img width="350" alt="Screenshot 2024-04-18 at 3 56 43 PM" src="https://github.com/user-attachments/assets/f627e1a2-7503-4f56-ae36-6702077ee7d9"> |<img width="350" alt="Screenshot 2024-04-18 at 3 56 43 PM" src="https://github.com/user-attachments/assets/d631bccd-f600-439b-9341-fe76ebbba2a1"> | #### Network Switch Confirm Pressed | **Interaction** | **Tracked event** | |--------------|--------------| | <img width="350" alt="Screenshot 2024-04-18 at 3 56 43 PM" src="https://github.com/user-attachments/assets/4b6508d0-c1e3-4dd9-8210-5e63d75b65c0"> |<img width="350" alt="Screenshot 2024-04-18 at 3 56 43 PM" src="https://github.com/user-attachments/assets/ecfc708c-b7e2-4530-9fe7-11740c293b12"> | <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). 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.
devin-ai-integration
bot
dismissed stale reviews from Cal-L and smilingkylan
via
eada60c
devin-ai-integration
bot
added
the
skip-sonar-cloud
|
Triggering CI rerun |
…-{{random_number_3_digits}}
smilingkylan
approved these changes
Oct 10, 2024
|
smilingkylan
deleted the
convert-ts-ganache-contract-address-registry-{{random_number_3_digits}}
branch
github-actions
bot
removed
the
needs-dev-review
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Converted the file to TypeScript and removed return types from methods.
This Devin run was requested by Jeffrey.
If you have any feedback, you can leave comments in the PR and I'll address them in the app!