Flag to create empty changeset #187
Conversation
🦋 Changeset is good to goLatest commit: 9a525ab We got this. 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 |
There was a problem hiding this comment.
Would be good to get the two more tests I mentioned in my review comments. The implementation itself looks solid.
- add two tests mentioned in comments
In addition, can we update the README.md to both mention the new flag in add, and explain why you would use this (I'd specifically call out that you likely only need to do this if you have CI set up that blocks merging without a changeset).
- update readme
- linting
Final task:
- Add a changeset 😄
| @@ -6,6 +6,7 @@ export type CliOptions = { | |||
| verbose?: boolean; | |||
| output?: string; | |||
| otp?: string; | |||
| empty?: boolean; | |||
There was a problem hiding this comment.
Minor note for learning: empty is actually always a boolean.
Why?
Because there is no 'default' option provided.
We have caught ourselves historically by effectively using 'undefined' as a distinct state for some of these. Here, we don't need a default (we will get false as the default for boolean thanks to meow).
All that said, if you need to update the PR and want to change this, go ahead, but I'm happy to merge this as is.
| // @ts-ignore if this is undefined, we have already exited | ||
| await add(cwd, config); | ||
| await add(cwd, { empty }, config); |
There was a problem hiding this comment.
Good catch on this being the idiomatic way we were adding these config bits. I'm going to leave a second comment which I don't expect actioned in this PR.
@mitchellhamilton flagging your here as food-for-thought code tidy - it might be worth making these commands pass config, then... cliConfig(?) as a general pattern, allowing us to provide a default for the third argument of the functions and making tests where you just want the defaults easier. Very low priority thought though.
| summary: "" | ||
| }; | ||
| // @ts-ignore | ||
| const call = writeChangeset.mock.calls[0][0]; |
There was a problem hiding this comment.
Can we add a test to writeChangeset
that tests that when passed this, the write function behaves as intended?
| releases = Object.entries(yamlStuff).map(([name, type]) => ({ | ||
| name, | ||
| type | ||
| })); |
There was a problem hiding this comment.
I can see you added a test to the 'read' package below - it would be good to add one to 'parse', as this is the place where the change is made (the place in charge of the concern as it were).
Code otherwise looks good.
Added the
--emptyflag to the add command, creates an empty changeset:yarn changeset add --emptyandyarn changeset --emptyNeeded to change the regex for parsing changeset files because it assumed existing releases and summary. Also added tests for both creating and reading an empty changeset.
Resolves #145