-
Notifications
You must be signed in to change notification settings - Fork 252
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
Improve error message when attempting to publish without publish script defined #415
Conversation
🦋 Changeset detectedLatest commit: 22e3ab1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ac76f06
to
add1e0c
Compare
.changeset/breezy-garlics-bathe.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
"@changesets/action": minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO the proposed change is a major change. It's a substantial change in the default behavior. If anybody is relying on publish being an opt-in then it could have a devastating effect for them if we suddenly try to publish their packages out of a blue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, like I'm personally using it in places where publishing is handled separately where I definitely wouldn't want this behaviour and the README documents a Without Publishing
workflow which with this PR isn't accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks! I'd never used this without publishing, so sorry I missed that was a way to use this. I've updated this PR to revert to the previous behavior but update the error message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can take another look if this is he best default behavior but that will have to be done as part of the next major release
add1e0c
to
35e09e9
Compare
Prior to this PR it would tell you
No changesets found
if you omitted thepublish
script, and it was very hard to figure out what to do. It was also misleading as the only reason there were no changesets present is because they were deleted earlier in the release process