Remove published-packages output in favor of publish-output#689
Remove published-packages output in favor of publish-output#689Andarist wants to merge 2 commits into
published-packages output in favor of publish-output#689Conversation
|
|
|
||
| if (result.published) { | ||
| core.setOutput("published", "true"); | ||
| core.setOutput( |
There was a problem hiding this comment.
Why I'd like to propose changing this? Currently, we interpret "git-tag" events as "releases". But with --no-git-tag and other customizations that's not exactly particularly accurate.
We should be emitting some other events for npm-only releases and allow the action to interpret them as releases. That's something to fix on the CLI side of things.
But even once we do it... the user wouldn't be able to tell exactly what and how got released. We'd have to "interpret" a mix of tag/npm releases into this array of published-packages. So my thinking is that by allowing the user to interpret the NDJSON output we can just satisfy more use cases without hardcoding some decisions into the action's core.
There was a problem hiding this comment.
i think we should have both.
having to parse and handle the CLI output requires quite a lot more than just checking if the published value is true, especially in CI flows.
since we also have the output file now, we can change the meaning of published:
published: was any package published
we could also add tagged
tagged: was any package tagged
There was a problem hiding this comment.
I quite like having published and tagged separated. Then we could let them decide how to handle it. Maybe also if publish passes but tag failed (or vice versa) it would be clearer which packages is affected
| A JSON array to present the published packages. The format is `[{"name": "@xx/xx", "version": "1.2.0"}, {"name": "@xx/xy", "version": "0.8.9"}]` | ||
| description: A boolean value to indicate whether any release was published | ||
| output: | ||
| description: The path to the Changesets publish output file in NDJSON format |
There was a problem hiding this comment.
| description: The path to the Changesets publish output file in NDJSON format | |
| description: The path to the publish output NDJSON file |
bluwy
left a comment
bluwy
left a comment
There was a problem hiding this comment.
I'm a bit concerned of the DX this would affect. Now because it's a path, they'd need an extra step to parse it in order to use the values, eg using this value in an if condition will be slightly annoying to set up.
Plus, it's not clear what the values of the ndjson are. We currently don't document it in core either (I assume it'll be stable but internal-ish) so this would result people to trail-and-error the process, which isn't nice.
I think I still prefer keeping the old output though, exposing a simplified structure for now.
3 participants
No description provided.