Skip to content

Conversation

@fgreinacher
Copy link
Contributor

@fgreinacher fgreinacher marked this pull request as ready for review January 13, 2023 16:05
@fgreinacher fgreinacher requested a review from travi January 13, 2023 16:06
Copy link
Member

@travi travi left a comment

Choose a reason for hiding this comment

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

looks great!

what would you think about releasing this as a beta before promoting to stable? we just released a beta of the npm plugin so we could try it in some real releases first since we havent fully proven out that core is ready to load esm plugins quite yet

@travi
Copy link
Member

travi commented Jan 14, 2023

what would you think about releasing this as a beta before promoting to stable? we just released a beta of the npm plugin so we could try it in some real releases first since we havent fully proven out that core is ready to load esm plugins quite yet

in our attempt to release a beta of core that included the beta of the npm plugin, we confirmed that we needed another iteration to handle loading esm plugins properly. until we promote that fix to stable, i think i'd advise against releasing this as stable quite yet

@fgreinacher fgreinacher changed the base branch from master to beta January 16, 2023 07:03
@fgreinacher
Copy link
Contributor Author

what would you think about releasing this as a beta before promoting to stable? we just released a beta of the npm plugin so we could try it in some real releases first since we havent fully proven out that core is ready to load esm plugins quite yet

Good idea, I rebased this against a beta branch. This should be all that's necessary to publish a beta release, right?

@fgreinacher fgreinacher requested a review from travi January 16, 2023 07:04
@travi
Copy link
Member

travi commented Jan 16, 2023

I rebased this against a beta branch. This should be all that's necessary to publish a beta release, right?

Yep that'll do it

@fgreinacher fgreinacher merged commit d6476f0 into beta Jan 16, 2023
@fgreinacher fgreinacher deleted the feat/esm branch January 16, 2023 07:09
@github-actions
Copy link

🎉 This PR is included in version 9.6.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@travi
Copy link
Member

travi commented Jan 16, 2023

🎉 great to see this make it's way out. Thanks for moving this forward, @fgreinacher!

One thing that I would highlight is that the esm transition should be considered breaking, but looks like it wasn't released that way in this case. Could you re-release to call out that breaking change? That way when promoted to stable the release will cover it then too. Also, when we do promote to stable, I recommend a normal merge so the releases from the pre-release branch are preserved.

@fgreinacher
Copy link
Contributor Author

Thanks for your thoughts @travi! Yea, I noticed that I forgot the breaking change trailer. What’s the best way to perform a re-release? Delete the tag and force-push?

Regarding the merge strategy: Currently this repo is configured to enforce squash merge. Could you change that setting?

@travi
Copy link
Member

travi commented Jan 17, 2023

What’s the best way to perform a re-release? Delete the tag and force-push?

yeah, all options are a bit ugly, but i that approach should work fine

Currently this repo is configured to enforce squash merge. Could you change that setting?

i updated to enable squash or normal merges, so you should be good to go once you're ready to promote to stable. just a reminder that you'll likely want to wait until one of the options mentioned here (promoting the current core beta to stable, or backporting the handling of esm plugins to core) make their way to the stable version of core.

fgreinacher added a commit that referenced this pull request Jan 18, 2023
BREAKING CHANGE: This package is now ESM-only and cannot be used by CJS packages.

BREAKING CHANGE: The minimum supported Node version has been changed to 18.0.0.
fgreinacher added a commit that referenced this pull request Jan 26, 2023
BREAKING CHANGE: This package is now ESM-only and cannot be used by CJS packages.

BREAKING CHANGE: The minimum supported Node version has been changed to 18.0.0.
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.

2 participants