Add select-mode, version, pack and publish sub-actions#656
Conversation
🦋 Changeset detectedLatest commit: 1c57c8c 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 |
Andarist
force-pushed
the
new-version-publish-subaction
branch
from
b6da0bd to
10ef577
Compare
Andarist
changed the title
select-mode, version, pack and publish sub-actions
| script: | ||
| description: "The command to use to publish packages" | ||
| required: false | ||
| packed-artifact-id: |
There was a problem hiding this comment.
is this a good input name? maybe pack-dir-artifact-id would be better?
There was a problem hiding this comment.
If we're to keep this, I think pack-artifact-id is a bit nicer.
There was a problem hiding this comment.
I'm OK with that too - I just thought about pack-dir-artifact-id as that would be symmetric~ with --from-pack-dir
There was a problem hiding this comment.
Oh, in that case let's go with pack-dir-artifact-id then so it's consistent. Maybe we want to revisit the same naming for the publish plan too
There was a problem hiding this comment.
Maybe we want to revisit the same naming for the publish plan too
I thiiink that's already consistent enough - --from-plan/publish-plan-artifact-id. But I guess we could want to consistently use "publish plan" or just "plan" in both of those. I don't mind the current naming scheme but I would also be OK with adjusting it.
There was a problem hiding this comment.
Yeah I mean --from-plan could probably be --from-publish-plan, but it's kinda long. But, probably good to be explicit and it's not a common flag to manually use too
There was a problem hiding this comment.
Ok, I’m starting that rename here: changesets/changesets#2108
| }); | ||
| }; | ||
|
|
||
| if (script) { |
There was a problem hiding this comment.
Note how with a custom publish script we don't handle fromPackDir at all. My best idea to handle this would be to also add support for passing this argument~ through CLI args. That way the scripts could forward env to the underlying changesets publish kinda naturally - without messing with flags forwarding. Thoughts?
There was a problem hiding this comment.
I'd prefer not relying on envs as it can be sometimes hard to follow who set what at when. Maybe we can support some sort of interpolation? Like script: pnpm format && pnpm changeset publish $action_args or ... publish --from-pack-dir $action_from_pack_dir. Just need to make sure to prevent potential script injections.
OR do we even need to allow specifying a script now? Since the sub-actions are explicit invocations now, if they need a pre-command or post-command, just do it in a pre-post-step. If they need a different version/publishing scheme, then don't call our action.
But I think what you have now is also fine and then we extend it later.
There was a problem hiding this comment.
OR do we even need to allow specifying a script now? Since the sub-actions are explicit invocations now, if they need a pre-command or post-command, just do it in a pre-post-step.
Yeah, I also wondered about that. Removing it would allow us to gather actual use cases for custom scripts before we re-commit to supporting them.
That said, it would break changesets/action's own publishing pipeline - or at least, it would make it more cumbersome~. But perhaps it wouldn't be too bad if only we'd expose changesets/action/github-release
There was a problem hiding this comment.
For changesets/action, I think our publish script could be called directly like before without using the sub-action. For version though, I guess we need like a pre/post hook to make changes before we commit 🤔
There was a problem hiding this comment.
For now, we can stick to using the old root action - and figure this one later.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
2 participants
Adds a very simple implementation of
/select-mode,/version, and/publishsub-actions that reuses functions from the root action. I renamed some of the inputs, copied most of the logic over. There's probably more cleanup possible but I don't want to stray too far for now.I also did not write any readme/docs as
I'm lazymaybe it changes after the publish-plan PRs.I made this PR mainly so we can make updates on it easier later, but I'm also fine closing this as starting from scratch with the publish-plan stuff if that's easier.