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

feat(cli): support json configuration for tokens create #2847

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

unekinn
Copy link
Contributor

@unekinn unekinn commented Nov 28, 2024

Closes #2846

Some things to note:

  • Most CLI options are no longer required, although they must be set either in json or through CLI. This could be confusing. I tried to look in to building options conditionally, so that e.g. options could be required if --json was not used or default config was not found, but this doesn't seem to be possible
  • The complete config is validated, so users will still get an error if options are missing. We could probably work on making the errors more clear.
  • CLI options take priority over json config. This is usually how CLI tools work.
  • Because CLI options take priority over json, we need to know whether an option is supplied or not. If we specify default values in the .options(...) call, there is no way to tell a default options (e.g. no --out-dir supplied) from a supplied option with the same value (--out-dir ./design-tokens). Therefore, default options are not specified there, but moved to the section "Create final props from defaults, json file and command-line options".

I wish CommanderJS had a built-in support for config files, since the default / options / json logic got pretty complex 😢

Copy link

changeset-bot bot commented Nov 28, 2024

⚠️ No Changeset found

Latest commit: e745bf4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Nov 28, 2024

Preview deployments for this pull request:

Storybook - 16. Dec 2024 - 13:48

@unekinn unekinn force-pushed the feat/cli-tokens-create-json-config branch from 807abb5 to 7561391 Compare November 28, 2024 13:15
Copy link
Contributor

github-actions bot commented Nov 28, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 55.14% 3188 / 5781
🔵 Statements 55.14% 3188 / 5781
🔵 Functions 86.6% 181 / 209
🔵 Branches 77.19% 518 / 671
File CoverageNo changed files found.
Generated in workflow #1303 for commit e745bf4 by the Vitest Coverage Report Action

@unekinn unekinn force-pushed the feat/cli-tokens-create-json-config branch from 7561391 to 0489d12 Compare December 16, 2024 11:03
@unekinn unekinn force-pushed the feat/cli-tokens-create-json-config branch from 0489d12 to e745bf4 Compare December 16, 2024 12:46
@unekinn unekinn marked this pull request as ready for review December 16, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration file for CLI
1 participant