Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 180.0.0 #4548

Merged
merged 31 commits into from
Jul 26, 2024
Merged

Release 180.0.0 #4548

merged 31 commits into from
Jul 26, 2024

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Jul 23, 2024

Explanation

This is the release candidate for v180.0.0:

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@MajorLift MajorLift requested a review from a team July 23, 2024 18:00
@MajorLift MajorLift self-assigned this Jul 23, 2024
Copy link

socket-security bot commented Jul 23, 2024

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/@metamask/[email protected], npm/@metamask/[email protected]

View full report↗︎

@MajorLift MajorLift linked an issue Jul 23, 2024 that may be closed by this pull request
1 task
@MajorLift MajorLift marked this pull request as ready for review July 23, 2024 19:31
@MajorLift MajorLift requested review from a team as code owners July 23, 2024 19:31
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya left a comment

Choose a reason for hiding this comment

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

Satisfied with the notifications side (@metamask/notification-services-controller, @metamask/profile-sync-controller)

matthewwalsh0
matthewwalsh0 previously approved these changes Jul 24, 2024
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Had some suggestions!

packages/assets-controllers/CHANGELOG.md Show resolved Hide resolved
packages/assets-controllers/CHANGELOG.md Show resolved Hide resolved
packages/assets-controllers/CHANGELOG.md Show resolved Hide resolved
packages/assets-controllers/CHANGELOG.md Show resolved Hide resolved
packages/polling-controller/CHANGELOG.md Show resolved Hide resolved
packages/queued-request-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/transaction-controller/CHANGELOG.md Show resolved Hide resolved
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we releasing this package? If not, should we add these changelog entries in another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To include the changes in #4556, I added releases for:

  • ens-controller (a3308b8)
  • gas-fee-controller (ac917f0)
  • selected-network-controller, queued-request-controller 61670f3
  • transaction-controller, user-operation-controller 773baa3

Every package with a diff in this PR is now being released (excluding json-rpc-engine).

- Upgrade TypeScript to v5.0 and set `moduleResolution` option to `Node16` ([#3645](https://github.com/MetaMask/core/pull/3645))
- Bump `@metamask/base-controller` from `^6.0.0` to `^6.0.2` ([#4517](https://github.com/MetaMask/core/pull/4517), [#4544](https://github.com/MetaMask/core/pull/4544))
- Bump `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.1` ([#3645](https://github.com/MetaMask/core/pull/3645))
- Bump peerDependency `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.0`
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are bumping a peer dependency then this is a breaking change as the client will also need to upgrade this dependency. Should we bump this package to 0.2.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Prithpal-Sooriya How would you feel about using a minor version bump to indicate that there are breaking changes in the release?

Copy link
Contributor Author

@MajorLift MajorLift Jul 25, 2024

Choose a reason for hiding this comment

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

Upgraded both {notifications-services,profile-sync}-controller to 0.2.0: f842953

@Prithpal-Sooriya Let me know if this goes against the team's intentions. I can simply revert that commit.

- Bump `@metamask/snaps-sdk` from `^4.2.0` to `^6.1.1` ([#3645](https://github.com/MetaMask/core/pull/3645), [#4547](https://github.com/MetaMask/core/pull/4547))
- Bump `@metamask/snaps-utils` from `^7.4.0` to `^7.8.1` ([#3645](https://github.com/MetaMask/core/pull/3645), [#4547](https://github.com/MetaMask/core/pull/4547))
- Bump `@metamask/utils` from `^8.3.0` to `^9.1.0` ([#4516](https://github.com/MetaMask/core/pull/4516), [#4529](https://github.com/MetaMask/core/pull/4529))
- Bump peerDependency `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.0` ([#3645](https://github.com/MetaMask/core/pull/3645))
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're bumping a peer dependency by a major then this would be a breaking change as it forces the client to also upgrade this package. Should we bump the version to 18.0.0?

Copy link
Contributor Author

@MajorLift MajorLift Jul 25, 2024

Choose a reason for hiding this comment

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

Good point. Fixed here! 2e13b78

@OGPoyraz
Copy link
Member

OGPoyraz commented Jul 25, 2024

Hi there! @MajorLift
May I ask if we can include a3bfbce into this release?
This is fairly simple one adds a public method to NFTController.

@MajorLift MajorLift dismissed stale reviews from matthewwalsh0 and Prithpal-Sooriya via 83060c1 July 25, 2024 13:52
@MajorLift
Copy link
Contributor Author

@OGPoyraz Of course! Any commits merged into main while this PR is open will need to be added to this release in any case. Thanks for bringing this one to my attention.

Changelog updated here: 83060c1

@MajorLift MajorLift requested review from Prithpal-Sooriya, a team and matthewwalsh0 July 25, 2024 18:30
Comment on lines +18 to +20
- Bump `@metamask/snaps-controllers` from `^8.1.1` to `^9.3.1` ([#3645](https://github.com/MetaMask/core/pull/3645), [#4547](https://github.com/MetaMask/core/pull/4547))
- Bump `@metamask/snaps-sdk` from `^4.2.0` to `^6.1.1` ([#3645](https://github.com/MetaMask/core/pull/3645), [#4547](https://github.com/MetaMask/core/pull/4547))
- Bump `@metamask/snaps-utils` from `^7.4.0` to `^7.8.1` ([#3645](https://github.com/MetaMask/core/pull/3645), [#4547](https://github.com/MetaMask/core/pull/4547))
Copy link
Contributor Author

@MajorLift MajorLift Jul 25, 2024

Choose a reason for hiding this comment

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

@MetaMask/accounts-engineers Should any of these be peer dependencies (esp. snaps-controllers)?

Copy link
Member

Choose a reason for hiding this comment

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

Typically the snaps controller is added as a peer dependency, for the same reason as the other controllers. I don't see the others as being used as peer deps though.

Perhaps we can move the controller to be a peer dependency later, since it's a pre-existing issue.

@MajorLift MajorLift requested review from a team July 25, 2024 19:24
@MajorLift
Copy link
Contributor Author

MajorLift commented Jul 25, 2024

It's worth noting that once we have our codeowners file updated, a PR covering the same packages as this one will require 6 approvals (accounts, assets, confirmations, notifications, wallet-api-platform, wallet-framework) possibly multiple times throughout the review process.

One way to make this a little less painful might be to first ask for approval from codeowners who own more packages in the PR (wallet-framework, confirmations), before pinging codeowners who own less packages.

mcmire
mcmire previously approved these changes Jul 25, 2024
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Left a few corrections mostly minor

packages/assets-controllers/CHANGELOG.md Show resolved Hide resolved
packages/message-manager/CHANGELOG.md Outdated Show resolved Hide resolved
packages/network-controller/CHANGELOG.md Show resolved Hide resolved
packages/profile-sync-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/profile-sync-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/selected-network-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/selected-network-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/signature-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/transaction-controller/CHANGELOG.md Show resolved Hide resolved
packages/user-operation-controller/package.json Outdated Show resolved Hide resolved
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

### Added

- Add and export object `USER_STORAGE_SCHEMA`, function `getFeatureAndKeyFromPath`, and type `UserStoragePath` ([#4543](https://github.com/MetaMask/core/pull/4543))
- Add `connectSnap` method to the `JwtBearerAuth` class for connecting to snap after initializing the Profile Sync SDK ([#4560](https://github.com/MetaMask/core/pull/4560))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Prithpal-Sooriya I added an entry for the new commit adding connectSnap.

@MajorLift MajorLift merged commit a429a95 into main Jul 26, 2024
116 checks passed
@MajorLift MajorLift deleted the release/180.0.0 branch July 26, 2024 15:23
AugmentedMode pushed a commit that referenced this pull request Jul 30, 2024
This is the release candidate for `v180.0.0`:
- `@metamask/[email protected]` (major)
- `@metamask/[email protected]` (major)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (minor)
- `@metamask/[email protected]` (minor)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (minor)
- `@metamask/[email protected]` (major)
- `@metamask/[email protected]` (major)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)
- `@metamask/[email protected]` (patch)

- Closes #3651
- Unblocks new releases in core.

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Elliot Winkler <[email protected]>
Co-authored-by: Mark Stacey <[email protected]>
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.

Upgrade to TypeScript v5.0
6 participants