Add options to configure snapshot preid format #830
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0ef05bd:
|
jakubmazanec
force-pushed
the
add-timestamp-separator
branch
from
36dd7a0
to
0ef05bd
Compare
🦋 Changeset detectedLatest commit: 0ef05bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
| "@changesets/types": minor | ||
| --- | ||
|
|
||
| Adds two options to configure snapshot preid format: which character separates timestamp part from the rest of the preid, and whether the timestamp part comes first in the preid. |
There was a problem hiding this comment.
In general, I'm not a fan of adding so many different options for something that feels very related - it seems like we are aiming to tweak a "format"/pattern of the generated snapshot with both of those.
I might give some alternative suggestions for this later on.
There was a problem hiding this comment.
Sure, I'm open to alternative way how to configure this, because TBH I don't like these two options either...
There was a problem hiding this comment.
I'm thinking about single option snapshotPreidTemplate, that would serve as a template for the snapshot preid, e.g. '{{label}}.{{timestamp}}'; placeholders would be replaced with actual timestamp and value of snapshot CLI option. But the problem is that snapshot option returns string | boolean, i.e. the label is optional, and that needs little more complex templating logic than simple placeholder replacement.
There was a problem hiding this comment.
And now I'm thinking that I would actually like to have different templates for the preid for different prereleases. So maybe it would be better to create a new CLI option rather than config option? 🤔
|
Could you refresh my memory on the motivation behind this being configurable? I know some~ relevant context but would love to hear out your arguments first. |
AFAIK the reason for having the timestamp first in the preid is to be able to predictably sort prereleases, and using
|
|
A competing PR has been created, you might find it interesting: #858 |
|
The mentioned completing PR has been merged in and thus we can close this PR. Nevertheless, thank you for your contribution! It's very much appreciated. |
I took the liberty to implement feature that was requested in some opened issues (can't remember which): to allow
.to separate timestamp part of the snapshot preid, and to allow timestamp part to come first.