Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add options to configure snapshot preid format #830

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/pink-eagles-agree.md
@@ -0,0 +1,7 @@
---
"@changesets/assemble-release-plan": minor
"@changesets/config": minor
"@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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'm open to alternative way how to configure this, because TBH I don't like these two options either...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? 🤔

8 changes: 8 additions & 0 deletions docs/config-file-options.md
Expand Up @@ -156,3 +156,11 @@ You would specify our github changelog generator with:
```

For more details on these functions and information on how to write your own see [changelog-functions](./modifying-changelog-format.md)

## `snapshotTimestampSeparator` (`'-'` or `'.'`)

This option sets which character is used for separating timestamp part from the rest of the preid when doing [snapshot releases](./snapshot-releases.md), i.e. `0.0.0-bulbasaur-THE_TIME_YOU_DID_THIS` `vs 0.0.0-bulbasaur.THE_TIME_YOU_DID_THIS`. The default is `-`.

## `snapshotTimestampPosition` (`start` or `end`)

This option sets whether timestamp part comes at the start or at the end of the preid when doing [snapshot releases](./snapshot-releases.md), i.e. `0.0.0-bulbasaur-THE_TIME_YOU_DID_THIS` `vs 0.0.0-THE_TIME_YOU_DID_THIS-bulbasaur`. The default is `end`.
36 changes: 36 additions & 0 deletions packages/apply-release-plan/src/index.test.ts
Expand Up @@ -48,6 +48,8 @@ class FakeReleasePlan {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -85,6 +87,8 @@ async function testSetup(
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -485,6 +489,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -548,6 +554,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -697,6 +705,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -782,6 +792,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -859,6 +871,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -936,6 +950,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1016,6 +1032,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1101,6 +1119,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1178,6 +1198,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1255,6 +1277,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1336,6 +1360,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: true,
Expand Down Expand Up @@ -1498,6 +1524,8 @@ describe("apply release plan", () => {
null
],
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1603,6 +1631,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "patch",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1688,6 +1718,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "minor",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1777,6 +1809,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "minor",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down Expand Up @@ -1880,6 +1914,8 @@ describe("apply release plan", () => {
access: "restricted",
baseBranch: "main",
updateInternalDependencies: "minor",
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
ignore: [],
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
Expand Down
124 changes: 101 additions & 23 deletions packages/assemble-release-plan/src/index.test.ts
Expand Up @@ -31,31 +31,109 @@ describe("assemble-release-plan", () => {
});
});

it("should assemble release plan for basic setup with snapshot", () => {
let { releases } = assembleReleasePlan(
setup.changesets,
setup.packages,
defaultConfig,
undefined,
true
);
test.each([
{
snapshot: true,
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: undefined,
resultRegexp: /0\.0\.0-\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: undefined,
resultRegexp: /0\.0\.0-foo-\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "." as const,
snapshotTimestampPosition: undefined,
resultRegexp: /0\.0\.0-foo\.\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "-" as const,
snapshotTimestampPosition: undefined,
resultRegexp: /0\.0\.0-foo-\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: "start" as const,
resultRegexp: /0\.0\.0-\d{14}-foo/
},
{
snapshot: "foo",
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: "end" as const,
resultRegexp: /0\.0\.0-foo-\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "-" as const,
snapshotTimestampPosition: "start" as const,
resultRegexp: /0\.0\.0-\d{14}-foo/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "-" as const,
snapshotTimestampPosition: "end" as const,
resultRegexp: /0\.0\.0-foo-\d{14}/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "." as const,
snapshotTimestampPosition: "start" as const,
resultRegexp: /0\.0\.0-\d{14}\.foo/
},
{
snapshot: "foo",
snapshotTimestampSeparator: "." as const,
snapshotTimestampPosition: "end" as const,
resultRegexp: /0\.0\.0-foo\.\d{14}/
},
{
snapshot: true,
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: "start" as const,
resultRegexp: /0\.0\.0-\d{14}/
},
{
snapshot: true,
snapshotTimestampSeparator: undefined,
snapshotTimestampPosition: "end" as const,
resultRegexp: /0\.0\.0-\d{14}/
}
])(
"should assemble release plan for basic setup with %j",
({
snapshot,
snapshotTimestampSeparator,
snapshotTimestampPosition,
resultRegexp
}) => {
let config = { ...defaultConfig };

if (snapshotTimestampSeparator) {
config.snapshotTimestampSeparator = snapshotTimestampSeparator;
}

if (snapshotTimestampPosition) {
config.snapshotTimestampPosition = snapshotTimestampPosition;
}

expect(releases.length).toBe(1);
expect(/0\.0\.0-\d{14}/.test(releases[0].newVersion)).toBeTruthy();
});

it("should assemble release plan for basic setup with snapshot and tag", () => {
let { releases } = assembleReleasePlan(
setup.changesets,
setup.packages,
defaultConfig,
undefined,
"foo"
);
let { releases } = assembleReleasePlan(
setup.changesets,
setup.packages,
config,
undefined,
snapshot
);

expect(releases.length).toBe(1);
expect(/0\.0\.0-foo-\d{14}/.test(releases[0].newVersion)).toBeTruthy();
});
expect(releases.length).toBe(1);
expect(resultRegexp.test(releases[0].newVersion)).toBeTruthy();
}
);

it("should assemble release plan with multiple packages", () => {
setup.addChangeset({
Expand Down
32 changes: 26 additions & 6 deletions packages/assemble-release-plan/src/index.ts
Expand Up @@ -27,20 +27,36 @@ function getPreVersion(version: string) {
return preVersion;
}

function getSnapshotSuffix(snapshot?: string | boolean): string | undefined {
function getSnapshotSuffix(
snapshot: string | boolean | undefined,
timestampSeparator: "-" | ".",
timestampPosition: "start" | "end"
): string | undefined {
if (snapshot === undefined) {
return;
}

let dateAndTime = new Date()
const parts: string[] = [];
const timestamp = new Date()
.toISOString()
.replace(/\.\d{3}Z$/, "")
.replace(/[^\d]/g, "");
let tag = "";

if (typeof snapshot === "string") tag = `-${snapshot}`;
if (timestampPosition === "start") {
parts.push(timestamp);

return `${tag}-${dateAndTime}`;
if (typeof snapshot === "string") {
parts.push(snapshot);
}
} else {
if (typeof snapshot === "string") {
parts.push(snapshot);
}

parts.push(timestamp);
}

return `-${parts.join(timestampSeparator)}`;
}

function getNewVersion(
Expand Down Expand Up @@ -95,7 +111,11 @@ function assembleReleasePlan(
const preInfo = getPreInfo(changesets, packagesByName, config, preState);

// Caching the snapshot version here and use this if it is snapshot release
const snapshotSuffix = getSnapshotSuffix(snapshot);
const snapshotSuffix = getSnapshotSuffix(
snapshot,
config.snapshotTimestampSeparator,
config.snapshotTimestampPosition
);

// releases is, at this point a list of all packages we are going to releases,
// flattened down to one release per package, having a reference back to their
Expand Down
4 changes: 4 additions & 0 deletions packages/config/src/index.test.ts
Expand Up @@ -45,6 +45,8 @@ test("read reads the config", async () => {
updateInternalDependencies: "patch",
ignore: [],
bumpVersionsWithWorkspaceProtocolOnly: false,
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
updateInternalDependents: "out-of-range",
Expand All @@ -62,6 +64,8 @@ let defaults = {
baseBranch: "master",
updateInternalDependencies: "patch",
ignore: [],
snapshotTimestampSeparator: "-",
snapshotTimestampPosition: "end",
___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH: {
onlyUpdatePeerDependentsWhenOutOfRange: false,
updateInternalDependents: "out-of-range",
Expand Down