Skip to content

Dont fail changeset status command if there are no changed packages - #504

Merged
Andarist merged 10 commits into
changesets:masterfrom
vitorbal:dont-fail-status-when-no-packages-changed
Jan 13, 2021
Merged

Dont fail changeset status command if there are no changed packages#504
Andarist merged 10 commits into
changesets:masterfrom
vitorbal:dont-fail-status-when-no-packages-changed

Conversation

@vitorbal

@vitorbal vitorbal commented Dec 2, 2020

Copy link
Copy Markdown
Contributor

Fixes #496.

Add a check to the status command, so process.exit(1) is only called if there are no changesets and a package has been changed. This prevents a few false-positives that can hurt DX. See #496 for more details.

@Andarist I wasn't sure if you wanted this to be a major bump or not.. IMO this should be a minor bump, since the new version will produce less errors than before, not more (meaning, no CI checks should fail that were not already failing before). Let me know if you agree or not, and I'll add the changeset accordingly.

@atlassian-cla-bot

atlassian-cla-bot Bot commented Dec 2, 2020

Copy link
Copy Markdown

Hooray! All contributors have signed the CLA.

@changeset-bot

changeset-bot Bot commented Dec 2, 2020

Copy link
Copy Markdown

馃 Changeset detected

Latest commit: 23afb73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/cli Minor

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

@Andarist Andarist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO this should be a minor bump, since the new version will produce less errors than before, not more (meaning, no CI checks should fail that were not already failing before)

This sounds reasonable to me. Any objections about this one @mitchellhamilton @Noviny ?

Comment thread packages/cli/src/commands/status/__tests__/status.ts
Comment thread packages/cli/src/commands/status/__tests__/status.ts Outdated
@emmatown

emmatown commented Dec 3, 2020

Copy link
Copy Markdown
Member

Sounds reasonable to me too but I wouldn't mind getting @Noviny's thoughts on it because I've never used changeset status to enforce having changesets.

Comment thread packages/cli/src/commands/add/__tests__/add.ts Outdated
Comment thread packages/cli/src/commands/add/createChangeset.ts
@vitorbal
vitorbal force-pushed the dont-fail-status-when-no-packages-changed branch from 4658caf to eb4f31a Compare December 29, 2020 13:32

const sinceBranch =
since === undefined ? (sinceMaster ? "master" : undefined) : since;
const releasePlan = await getReleasePlan(cwd, sinceBranch, config);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm wondering - do you happen to know why this doesn't account for config.baseBranch? 馃 I see that it's utilized by @changesets/read but not sure why. Maybe just something you have figured out while working on this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question! I took a look at the original PR that added it and couldn't figure out the intent here, so I decided to follow the same logic. Maybe @mitchellhamilton remembers?

@Andarist Andarist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a changeset for this? If not - I can do it myself later and merge it then.

@vitorbal vitorbal left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the hiatus @Andarist, I was taking a few days off! I'd be glad to add the changeset. I'll add a minor bump as per our conversation earlier in this PR.


const sinceBranch =
since === undefined ? (sinceMaster ? "master" : undefined) : since;
const releasePlan = await getReleasePlan(cwd, sinceBranch, config);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question! I took a look at the original PR that added it and couldn't figure out the intent here, so I decided to follow the same logic. Maybe @mitchellhamilton remembers?

Comment thread .changeset/chatty-plums-watch.md Outdated
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.

changeset status errors when no publishable package has been modified

3 participants