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

Add release types for major and minor prereleases #944

Merged

Conversation

kjellknapen
Copy link
Contributor

Allow user to do prereleases of major and minor releases. It's very weird that isn't a possibility right now.

@ecraig12345
Copy link
Member

ecraig12345 commented Feb 27, 2024

This makes sense but would probably be perceived as a breaking change in a lot of repos that use beachball today, since suddenly several new change types would be available and have to be manually added to their disallowedChangeTypes if not desired (many repos disable prerelease changes today). I made an issue #947 to track adding this whenever we do a new major version, though we don't have specific plans for timing yet.

In the meantime, I think a workaround would be to revert the promptForChange additions, but keep the new SortedChangeTypes. This would allow the new types to be specified as --type on the command line (or by editing the change file) and ensure they're handled with proper precedence. And then if you wanted to include the types in your repo's prompt, you could implement a custom changeFilePrompt in beachball.config.js.

@kjellknapen
Copy link
Contributor Author

kjellknapen commented Mar 5, 2024

@ecraig12345 Alright, I removed the other pre* versions from the prompt. Is that something that can be added in a minor/patch release in this way or do I need to make other changes to ensure we don't break current implementation?

I could optionally also disallow them all if prerelease is disallowed, but that may be a hacky solution?

@ecraig12345
Copy link
Member

@kjellknapen I think it's fine if new change types are allowed by default as long as they don't show up in the prompt.

I ran a build and it looks like you'll also need to update src/changelog/renderPackageChangelog.ts groupNames with the new types. Those are used as headers in the CHANGELOG.md if any changes of that type are present.

@kjellknapen
Copy link
Contributor Author

@ecraig12345 Done! Thanks for the quick response. I used the same title as the respective versions but added (pre-release) behind them.

@ecraig12345
Copy link
Member

Looks like you accidentally installed with pnpm (this repo uses yarn)--can you delete the pnpm lock file?

@kjellknapen
Copy link
Contributor Author

Oops my bad, was working in a different project with pnpm, so I was in that flow. I removed it

ecraig12345
ecraig12345 previously approved these changes Mar 6, 2024
@ecraig12345
Copy link
Member

Oh I guess you need a change file too.

ecraig12345
ecraig12345 previously approved these changes Mar 7, 2024
@kjellknapen
Copy link
Contributor Author

I guess I need to update some tests aswell

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ecraig12345 Does the test failing here signal that my changes are still breaking?

@kjellknapen
Copy link
Contributor Author

@ecraig12345 Had any time to review this once more? Tests should work now

@kjellknapen kjellknapen requested a review from ecraig12345 April 9, 2024 06:07
@ecraig12345 ecraig12345 merged commit d99780c into microsoft:master Apr 9, 2024
4 checks passed
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.

2 participants