Skip to content

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Dec 4, 2024

  • Move @metamask/gas-fee-controller and @metamask/network-controller from peerDependencies to dependencies, as neither of them are being used for communication
  • Remove unused dev dependencies @metamask/composable-controller, @types/readable-stream, jest-environment-jsdoc, and ts-node as they were not being used
  • Replace rimraf with a shell command

This is a cleanup task that I'd like to get in before releasing the next major version.

Copy link

socket-security bot commented Dec 4, 2024

- Move `@metamask/gas-fee-controller` and `@metamask/network-controller`
  from `peerDependencies` to `dependencies`, as neither of them are
  being used for communication
- Remove unused dev dependencies `@metamask/composable-controller`,
  `@types/readable-stream`, `jest-environment-jsdoc`, and `ts-node` as
  they were not being used
- Replace `rimraf` with a shell command
@mcmire mcmire force-pushed the clean-up-dependencies branch from 1bfc2d6 to e94c749 Compare December 4, 2024 22:32
@mcmire mcmire marked this pull request as ready for review December 4, 2024 22:35
@mcmire mcmire requested review from a team December 4, 2024 22:35
@mcmire mcmire self-assigned this Dec 4, 2024
@mcmire mcmire merged commit 9bcc4e9 into main Dec 5, 2024
11 checks passed
@mcmire mcmire deleted the clean-up-dependencies branch December 5, 2024 22:24
"lint:changelog": "auto-changelog validate --prettier",
"lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write",
"build:clean": "rimraf dist && yarn build",
"build:clean": "rm -rf dist && yarn build",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved just a question why are we replacing rimraf with rm -rf which wouldn't work with windows machines unless or course git bash or other tooling is used ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I can submit another to revert this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change reverted: #376

@Gudahtt
Copy link
Member

Gudahtt commented Dec 5, 2024

Move @metamask/gas-fee-controller and @metamask/network-controller from peerDependencies to dependencies, as neither of them are being used for communication

Hmm, unfortunately I don't think this is true, they are both used for communication. The former is used not via the messenger, but the fetchGasFeeEstimates constructor parameter. And the latter is used in the messenger directly, which you may have missed because it's in the types.ts file.

@mcmire
Copy link
Contributor Author

mcmire commented Dec 5, 2024

@Gudahtt Ah... you're right, thanks. I will add those packages back to dev + peerDependencies.

@Gudahtt Gudahtt mentioned this pull request Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants