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 236.0.0 #4870

Merged
merged 10 commits into from
Oct 30, 2024
Merged

Release 236.0.0 #4870

merged 10 commits into from
Oct 30, 2024

Conversation

bergeron
Copy link
Contributor

This is intended to release the asset controllers. I also included other controllers the release tool thought should be included

@bergeron bergeron requested review from a team as code owners October 30, 2024 00:55
@bergeron
Copy link
Contributor Author

I'm considering simplifying this by just releasing assets-controllers, and intentionally skipping other packages. If anyone can help verify it's safe to do so.

Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

  • Most of these releases (other than @metamask/assets-controllers itself) should be patch version bumps instead.
    • The only potential breaking changes in these changelogs are the major version bumps for @metamask/utils and @metamask/eth-sig-util (@metamask/{keyring-controller,message-manager}). These updates should be evaluated based on their effect on the packages, and if they are indeed breaking changes they should be noted as such in the changelogs. If not, the package should not be bumped by a major version.
    • For reference: https://github.com/MetaMask/core/blob/main/docs/reviewing-release-prs.md
  • Major version bumps in peer dependencies are always breaking changes, and should be recorded in the changelogs. Non-major bumps in peer dependencies should not be applied at all in the package.json peerDependencies section nor in the changelog. Updating the devDependencies entry is still necessary.
  • Version bumps applied by this PR should also be added to the changelogs.
  • The notifications team packages (@metamask/{notification-services,profile-sync}-controller) are pre-release (v0.x.x), so major releases should bump a "minor" version and minor/patch releases a "patch" version.

I'm considering simplifying this by just releasing assets-controllers, and intentionally skipping other packages. If anyone can help verify it's safe to do so.

When creating major releases, feature teams are expected to also publish unreleased changes in peer dependencies (especially for peerDeps being bumped) and dependents, and deliver them to the clients.

Previous lapses in these follow-up tasks have resulted in significant package version disparities in the clients, and we're working on tooling (https://github.com/MetaMask/MetaMask-planning/issues/2988) and guidelines to ensure that package versions will stay aligned in the future.

Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

typos

packages/accounts-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/keyring-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/message-manager/CHANGELOG.md Outdated Show resolved Hide resolved
packages/notification-services-controller/CHANGELOG.md Outdated 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
@FrederikBolding
Copy link
Member

@bergeron Can we release controller-utils as part of this as well?

@bergeron
Copy link
Contributor Author

Major version bumps in peer dependencies are always breaking changes, and should be recorded in the changelogs. Non-major bumps in peer dependencies should not be applied at all in the package.json peerDependencies section nor in the changelog. Updating the devDependencies entry is still necessary.

Version bumps applied by this PR should also be added to the changelogs.

Can you explain how this is different from what's in the PR? I'm just running the release tool and categorizing each entry it puts in the changelog. Is there something additional to do on top?

@bergeron
Copy link
Contributor Author

bergeron commented Oct 30, 2024

The only potential breaking changes in these changelogs are the major version bumps for @metamask/utils and @metamask/eth-sig-util

I wasn't sure how to determine whether these were breaking, so errred on the side of major release. But can mark that as breaking in changelog unless someone can say they're not breaking.

Looking deeper into the changes I think they're both non breaking for consumers

@bergeron bergeron requested review from a team as code owners October 30, 2024 16:47
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

We generally don't include dev dependency updates in changelogs, but they can be included for reference if relevant to the team.

packages/accounts-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/assets-controllers/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

This PR bumps @metamask/controller-utils from ^11.4.1 to ^11.4.2. This needs to be reflected in the changelogs.

packages/message-manager/CHANGELOG.md Outdated Show resolved Hide resolved
packages/network-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/polling-controller/CHANGELOG.md Outdated Show resolved Hide resolved
packages/preferences-controller/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your patience.

@bergeron bergeron merged commit a21acc6 into main Oct 30, 2024
119 checks passed
@bergeron bergeron deleted the release/236.0.0 branch October 30, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants