Address minor TODOs #55

zeyadkhaled · 2024-02-12T15:12:32Z

This PR addresses few TODOs

Parse config file based on extension (Protect against future inclusion of other extensions)
Add semVer sorting for version list command
Update isValidSemver utility to use the semver library
Update autoVersion to check for various git-describe failures and give user a better error message

changelog-app · 2024-02-12T15:12:37Z

Generate changelog in `packages/cli/changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

Address minor issues

Check the box to generate changelog(s)

Generate changelog entry

packages/cli/src/commands/site/deploy/siteDeployCommand.mts

packages/cli/src/util/autoVersion.ts

packages/cli/src/util/config.ts

tzyl · 2024-02-13T16:22:25Z

packages/cli/src/commands/site/version/list/versionListCommand.mts

-  for (const version of versions) {
+
+  const semver = await import("semver");
+  const sortedVersions = semver.rsort(versions.filter(v => semver.valid(v)));


We can probably assume the versions are enforced semver given the timelines?

I think its better to leave it and I could clarify with a comment. I personally have few broken apps that have LOOSE versions stored

Yeah I guess the point is that after a certain point in time in the future we're not going to have the control to filter this and it will be on the service layer to decide how to handle this instead

tzyl · 2024-02-13T16:28:13Z

packages/cli/src/util/autoVersion.ts

+  const version = gitVersion.trim().replace(prefixRegex, "");
+  if (!isValidSemver(version)) {
+    throw new ExitProcessError(
+      2,


From earlier comments we don't actually do anything with this errorCode information on the error and it's inconsistent on what it means where some of the net code uses it as a HTTP status code and here it seems like we're using it as an exit code. What should we do with it?

I actually think it doesn't add any value as well. So I could remove the passing of errorCode in a FLUP. Unless you foresee a usecase where we could benefit from it consistently?

packages/cli/src/util/autoVersion.ts

tzyl · 2024-02-14T13:16:21Z

packages/cli/src/commands/site/version/list/versionListCommand.mts

-  for (const version of versions) {
+
+  const semver = await import("semver");
+  const sortedVersions = semver.rsort(versions.filter(v => semver.valid(v)));


Yeah I guess the point is that after a certain point in time in the future we're not going to have the control to filter this and it will be on the service layer to decide how to handle this instead

packages/cli/src/util/autoVersion.ts

packages/cli/src/util/config.ts

tzyl · 2024-02-14T13:27:53Z

packages/cli/src/util/autoVersion.ts

-        `The version string ${version} is not SemVer compliant.`,
-      );
+    gitVersion = stdout;
+  } catch (error: any) {


Hmm, have you tested for this new execAsync change that it throws? If it does not throw on non-zero exit code is the error message on stderr instead?

Tested and throws 👍

…tions * origin/main: Update api-gateway definitions (#38) Update copyright to use new Date() (#66) fix generate type restrict object : add bulk no params options Autorelease Autorelease Update ExitProcessError tips and use option instead of argument (#63) Harmonize usage of options `foundryUrl` & `clientId` across all cli commands (#50) Address minor TODOs (#55) CLI error tips (#61) Include repository link in package.json (#60) Handle version/autoVersion selection in the parsing layer (#52) Add generated changelog entries Add helper Add generated changelog entries remove commented out code one more change add intersects Autorelease Refactor site link consola box code (#58) Prompt user for destructive commands (#56) Display site link after deploy (#57) Review CLI wording (#54) use helper function remove anys Fix bulk action type Add generated changelog entries one more refactor refactor bulk actions fix second attempt bulk actions

zeyadkhaled added 3 commits February 12, 2024 14:22

Add checks on config file extension for parsing

b32efc4

Add semver sorting

349e754

Update isValidSemver to use semver valid

04fe507

zeyadkhaled added 2 commits February 13, 2024 12:31

Update autoVersion to check for specific errors

b0ce931

Filter invalid semVer strings and reverse sort

cb13faa

zeyadkhaled changed the title ~~Address minor issues~~ Address minor TODOs Feb 13, 2024

zeyadkhaled marked this pull request as ready for review February 13, 2024 12:43

zeyadkhaled assigned tzyl Feb 13, 2024

zeyadkhaled commented Feb 13, 2024

View reviewed changes

packages/cli/src/commands/site/deploy/siteDeployCommand.mts Outdated Show resolved Hide resolved

tzyl reviewed Feb 13, 2024

View reviewed changes

CRs

dee086c

zeyadkhaled requested a review from tzyl February 14, 2024 12:04

Fix test

20aed4c

tzyl approved these changes Feb 14, 2024

View reviewed changes

zeyadkhaled and others added 2 commits February 14, 2024 14:06

CRs

fc00e95

Add generated changelog entries

76a2807

zeyadkhaled added the merge when ready label Feb 14, 2024

bulldozer-bot bot merged commit cfc1950 into main Feb 14, 2024
6 checks passed

bulldozer-bot bot deleted the zka/minor-todos branch February 14, 2024 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address minor TODOs #55

Address minor TODOs #55

zeyadkhaled commented Feb 12, 2024 •

edited

Loading

changelog-app bot commented Feb 12, 2024 •

edited by zeyadkhaled

Loading

tzyl Feb 13, 2024

zeyadkhaled Feb 14, 2024

tzyl Feb 14, 2024

tzyl Feb 13, 2024

zeyadkhaled Feb 14, 2024

tzyl Feb 14, 2024

tzyl Feb 14, 2024

zeyadkhaled Feb 14, 2024

Address minor TODOs #55

Address minor TODOs #55

Conversation

zeyadkhaled commented Feb 12, 2024 • edited Loading

changelog-app bot commented Feb 12, 2024 • edited by zeyadkhaled Loading

Generate changelog in packages/cli/changelog/@unreleased

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeyadkhaled commented Feb 12, 2024 •

edited

Loading

changelog-app bot commented Feb 12, 2024 •

edited by zeyadkhaled

Loading

Generate changelog in `packages/cli/changelog/@unreleased`