chore: upgrade oclif to v2 #161
Conversation
| @@ -6,7 +6,7 @@ import { KnockEnv } from "@/lib/helpers/const"; | |||
| import { withSpinner } from "@/lib/helpers/request"; | |||
| import { promptToConfirm } from "@/lib/helpers/ux"; | |||
|
|
|||
| export default class Commit extends BaseCommand { | |||
| export default class Commit extends BaseCommand<typeof Commit> { | |||
There was a problem hiding this comment.
This change allows us to type this.props more precisely for each command.
| description: Translation.translationRefDescription, | ||
| required: true, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
One of the main changes in oclif v2. Args are now defined with Args, just like for flags.
| required: true, | ||
| aliases: ["recipient"], | ||
| summary: | ||
| "One or more recipient ids for this workflow run, separated by comma.", | ||
| multiple: true, | ||
| delimiter: ",", |
There was a problem hiding this comment.
@meryldakin Sooo it turns out oclif v2 added a delimiter option that would parse the string value into an array, which is the same thing we were doing with commaSeparatedStr 😅 (but you have to set multiple: true).
I removed the helper in favor of using the built-in delimiter option, but wanted to give you a heads up!
There was a problem hiding this comment.
totally missed this!! gotcha thanks!
| // Global flags are inherited by any command that extends BaseCommand. | ||
| static globalFlags = { | ||
| // Base flags are inherited by any command that extends BaseCommand. | ||
| static baseFlags = { |
There was a problem hiding this comment.
globalFlags has been renamed to baseFlags.
| const expectedArgs = { | ||
| translationRef: undefined, | ||
| }; | ||
| const expectedArgs = {}; |
There was a problem hiding this comment.
It appears the behavior changed with args when an arg was defined but not provided — before we used to get {arg: undefined} vs nothing at all.
thomaswhyyou
requested review from
brentjanderson,
cjbell,
juanazam,
bceskavich and
meryldakin
| @@ -96,7 +100,7 @@ export default class WorkflowGet extends BaseCommand { | |||
| this.log(""); | |||
|
|
|||
| if (workflow.steps.length === 0) { | |||
| return CliUx.ux.log(" This workflow has no steps to display."); | |||
| return ux.log(" This workflow has no steps to display."); | |||
There was a problem hiding this comment.
Nit:
| return ux.log(" This workflow has no steps to display."); | |
| return ux.log("This workflow has no steps to display."); |
There was a problem hiding this comment.
Ah the leading space is there intentionally to align the left spacing in the display. Will add a quick comment.
Description
Oclif, the cli framework library we use, had a major version bump to v2 earlier this year.
It included a few breaking changes, which are explained in the migration guide here: https://github.com/oclif/core/blob/main/MIGRATION.md
Some notable changes captured in this PR:
Argsinstead of a list of objects (i.e. similar to how flags are configured).CliUxmodule is renamed toux.Propsinbase-command.ts.This PR contains no code changes outside of the oclif v2 upgrade.
Tasks
KNO-3195