Some updates #2
Some updates #2
Conversation
I think I've made things better but feel free to overrule any/all things
README.md
Outdated
| # The --otp=1 is to get around a bug in changesets | ||
| # It will be fixed in Changesets 2 | ||
| publish: yarn release --otp=1 | ||
| publish: yarn changeset publish |
There was a problem hiding this comment.
The original reasoning behind the publish option was to allow building before publish but this made me think more, I'm thinking this option should be removed and publish should happen if an NPM_TOKEN is provided and a build option should be added that's run before publish. Any thoughts on this?
There was a problem hiding this comment.
So was the idea it would run yarn release which could be their own aliased yarn command doing whatever? But it assumes that they added changeset publish somewhere in the release because we pass in the otp?
Agree that it should run a build thing. I wouldn't mind explicitly setting a publish boolean (or setting a publish command being the trigger for this) to make when the bot will publish more explicit than implicit.
Also, since we don't need to pass anything to the changeset command, it might be worth having it run yarn pre-publish and then the bot run the changeset command? It cuts the bot off from random publish scripts in non-changeset repos but otherwise seems fine.
There was a problem hiding this comment.
setting a publish command being the trigger for this
checked code to confirm this is what it does, will document loudly that the existence of this triggers this.
There was a problem hiding this comment.
So was the idea it would run yarn release which could be their own aliased yarn command doing whatever? But it assumes that they added changeset publish somewhere in the release because we pass in the otp?
Yes, that's correct. I know it's bad, it was a "this is a bad idea but I want this to work and doing this will make it work" thing.
Agree that it should run a build thing. I wouldn't mind explicitly setting a publish boolean (or setting a publish command being the trigger for this) to make when the bot will publish more explicit than implicit.
SGTM
Also, since we don't need to pass anything to the changeset command, it might be worth having it run yarn pre-publish and then the bot run the changeset command? It cuts the bot off from random publish scripts in non-changeset repos but otherwise seems fine.
As in make this action run a pre-publish script or the Changesets CLI?
README.md
Outdated
|
|
||
| ```yml | ||
| name: Release | ||
| name: Version |
There was a problem hiding this comment.
My thoughts were release is the right word because it's the combination of version + sometimes build + publish (changesets/changesets#158) any objections to that?
There was a problem hiding this comment.
I changed the one that was specifically 'copy this if you just want versioning' as opposed to the 'this one does versioning and commit' - to try and make the 'version' vs 'release' definition more consistent. Happy to move it back.
There was a problem hiding this comment.
Hmmm, that makes sense. My thoughts on naming the things release was that I was planning on renaming this action to changesets/release-action so having some of the things named version and some release would be strange.
Maybe we could have a version action and a publish action but separating the things because of naming seems like not the best idea?
There was a problem hiding this comment.
I think sticking with Release is simply going to be easier (I don't want to make this two actions if we can help it)
meta-discussion: Is it better to have one changesets GH action that's configurable with multiple actions, vs having different things for different tasks. The way this was initially set up, I thought we were going to be getting one package, but we've since divided. What's the reasons?
Moving this to an issue to not bike-shed on this PR, as I think the PR is otherwise good to go.
|
Closing as old |
I think I've made things better but feel free to overrule any/all things