Skip to content

2.0.0#111

Merged
Mrtenz merged 6 commits intoMetaMask:mainfrom
legobeat:release/2.0.0
Sep 7, 2023
Merged

2.0.0#111
Mrtenz merged 6 commits intoMetaMask:mainfrom
legobeat:release/2.0.0

Conversation

@legobeat
Copy link
Contributor

@legobeat legobeat commented Sep 7, 2023

This is the release candidate for version 2.0.0.

github-actions and others added 2 commits September 7, 2023 05:33
@legobeat legobeat requested a review from a team as a code owner September 7, 2023 05:42
@legobeat
Copy link
Contributor Author

legobeat commented Sep 7, 2023

@legobeat legobeat requested a review from Mrtenz September 7, 2023 07:13
Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>
@legobeat legobeat requested a review from Mrtenz September 7, 2023 07:30
@FrederikBolding
Copy link
Member

While this is technically a breaking change in the type I don't think it's really necessary to do a major bump for it. What do you think?

@Mrtenz
Copy link
Member

Mrtenz commented Sep 7, 2023

While this is technically a breaking change in the type I don't think it's really necessary to do a major bump for it. What do you think?

If it's breaking, technically or not, I think we should do a major bump. 😅

@legobeat
Copy link
Contributor Author

legobeat commented Sep 7, 2023

While this is technically a breaking change in the type I don't think it's really necessary to do a major bump for it. What do you think?

If we consider that this was a type-only change - TypeScript being a developer tool and strictly optional - I would tend to agree that breaking type interfaces shouldn't necessitate a breaking change.

However:

  1. Treating breaking TypeScript interfaces as semver-major is in line with current and recent practice in other repos in the MetaMask org. So it's a bit of a separate discussion.
  2. This does in fact change runtime behavior so it's breaking even if we disregard that IMO

I guess two options where both sound reasonable to me:

  1. revert Validate snap ids #75 -> release 1.3.0 -> re-apply Validate snap ids #75 -> release 2.0.0
  2. go ahead with 2.0.0 in one go as in this pr

@FrederikBolding
Copy link
Member

This does in fact change runtime behavior so it's breaking even if we disregard that IMO

Yeah, you are right. However, for our use we do not currently rely on the runtime behaviour. Which is why I brought it up.

But I am fine with following semver properly here as well. Just wanted to voice the option, because it wouldn't be breaking with our usage.

@Mrtenz Mrtenz merged commit a689dc9 into MetaMask:main Sep 7, 2023
legobeat added a commit to legobeat/snaps-registry that referenced this pull request Sep 7, 2023
This reverts commit a689dc9.

The release failed and will be rescheduled as-is.
Mrtenz pushed a commit that referenced this pull request Sep 7, 2023
This reverts commit a689dc9.

The release failed and will be rescheduled as-is.
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.

3 participants