chore(cli): function to turn yargs output to cliArguments #32696

kaizencc · 2024-12-31T15:16:08Z

This PR has no impact on CLI functionality

This PR generates a new function, convertToCliArgs, that takes in the output of the yargs configuration and turns it into a CliArguments type. This allows us to subsequently utilize the strongly-typed object in cli.ts, but this PR does not actually do that.

The ultimate benefit is that now we can ensure exactly what goes into this type. We can then enforce that any property allowed to be specified via cdk.json is also an option in the cli. When all my refactoring is done, there will be no options that are only specified via cdk.json, and no options only specified via CLI.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

codecov · 2024-12-31T15:29:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.96%. Comparing base (3b162fc) to head (7c310b2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32696   +/-   ##
=======================================
  Coverage   66.96%   66.96%           
=======================================
  Files         329      329           
  Lines       18663    18663           
  Branches     3258     3258           
=======================================
  Hits        12497    12497           
  Misses       5839     5839           
  Partials      327      327

Flag	Coverage Δ
suite.unit	`66.96% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.71% <ø> (ø)`
packages/aws-cdk-lib/core	`82.08% <ø> (ø)`

kaizencc · 2024-12-31T17:37:10Z

tools/@aws-cdk/cli-args-gen/lib/cli-args-function-gen.ts

+  });
+}
+
+function buildCliArgsFunction(config: CliConfig): string {


for all of the following buildXxx functions, I am sure there is a more idiomatic way of achieving what I want via typewriter. I have done it this way because it felt the simplest for me as of right now, and I believe it will be easier to refactor later with the benefit of knowing exactly what the generated function should look like.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

packages/aws-cdk/test/cli-arguments.test.ts

codecov.yml

iliapolo · 2025-01-01T09:10:29Z

packages/aws-cdk/lib/convert-to-cli-args.ts

+      break;
+
+    case 'acknowledge':
+      commandOptions = {};


Looking at our current for this command I see:

aws-cdk/packages/aws-cdk/lib/cli.ts

Lines 433 to 434 in fcf4ecd

case 'ack':

return cli.acknowledge(args.ID);

Should this PR also handle positional arguments? I would expect we have a strongly typed args.acknowledge.id or something.

Good point. A big part of my refactor is to make sure we don't have to use args, argv, and configuration.settings everywhere and instead just have 1 object with arguments.

Currently those positional arguments are found in index 1 and beyond on argv._: argv._: [Command, ...string]

It will be a bit awkward to figure out the right way to deal with that, which I would like to do in a separate PR.

packages/aws-cdk/scripts/cli-args-gen.ts

…nto conroy/commandenum

iliapolo · 2025-01-02T18:09:25Z

packages/aws-cdk/lib/cli-arguments.ts

@@ -2,6 +2,7 @@
 // GENERATED FROM packages/aws-cdk/lib/config.ts.
 // Do not edit by hand; all changes will be overwritten at build time from the config file.
 // -------------------------------------------------------------------------------------------
+// istanbul ignore file


Does this actually do something?

No. I forgot to yarn build aws-cdk to remove them...

aws-cdk-automation · 2025-01-02T18:13:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 7c310b2
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2025-01-02T18:14:22Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2025-01-02T18:16:38Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2025-01-02T18:16:53Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

kaizencc added 2 commits December 30, 2024 11:49

chore(cli): update Command enum

de4838f

chore(cli): function to turn yargs output to cliArguments

919c6d9

aws-cdk-automation requested a review from a team December 31, 2024 15:16

github-actions bot added the p2 label Dec 31, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Dec 31, 2024

kaizencc requested a deployment to test-pipeline December 31, 2024 15:17 — with GitHub Actions Waiting

aws-cdk-automation previously requested changes Dec 31, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Dec 31, 2024

Merge branch 'main' into conroy/commandenum

db0e89c

kaizencc requested a deployment to test-pipeline December 31, 2024 16:07 — with GitHub Actions Waiting

Merge branch 'main' into conroy/commandenum

a1bbfbf

kaizencc requested a deployment to test-pipeline December 31, 2024 16:43 — with GitHub Actions Waiting

generate convert func in new file

bc6ddfd

kaizencc requested a deployment to test-pipeline December 31, 2024 17:26 — with GitHub Actions Waiting

kaizencc marked this pull request as ready for review December 31, 2024 17:34

kaizencc requested a review from a team as a code owner December 31, 2024 17:34

kaizencc commented Dec 31, 2024

View reviewed changes

add test

d606b17

kaizencc added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Dec 31, 2024

kaizencc requested a deployment to test-pipeline December 31, 2024 17:41 — with GitHub Actions Waiting

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 31, 2024

Merge branch 'main' into conroy/commandenum

7f97067

kaizencc requested a deployment to test-pipeline December 31, 2024 21:51 — with GitHub Actions Waiting

update codecov to exclude generated files

795bf08

kaizencc requested a deployment to test-pipeline December 31, 2024 21:54 — with GitHub Actions Waiting

github-actions bot mentioned this pull request Jan 1, 2025

Monthly PR metrics report #32703

Open

iliapolo reviewed Jan 1, 2025

View reviewed changes

kaizencc and others added 3 commits January 2, 2025 11:22

test uses parseCommandLineArguments

1510105

Merge branch 'main' into conroy/commandenum

3002617

Merge branch 'conroy/commandenum' of https://github.com/aws/aws-cdk i…

a8b4187

…nto conroy/commandenum

kaizencc requested a deployment to test-pipeline January 2, 2025 16:33 — with GitHub Actions Waiting

use codecov ignore file

ce2b2e5

kaizencc requested a deployment to test-pipeline January 2, 2025 16:36 — with GitHub Actions Waiting

fix tests

be39bd3

kaizencc requested a deployment to test-pipeline January 2, 2025 17:09 — with GitHub Actions Waiting

jest ignore files

7c310b2

kaizencc requested a deployment to test-pipeline January 2, 2025 17:44 — with GitHub Actions Waiting

iliapolo approved these changes Jan 2, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 2, 2025

mergify bot merged commit efe7c2e into main Jan 2, 2025
22 of 23 checks passed

mergify bot deleted the conroy/commandenum branch January 2, 2025 18:16

github-actions bot locked as resolved and limited conversation to collaborators Jan 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cli): function to turn yargs output to cliArguments #32696

chore(cli): function to turn yargs output to cliArguments #32696

kaizencc commented Dec 31, 2024 •

edited

Loading

aws-cdk-automation left a comment

codecov bot commented Dec 31, 2024 •

edited

Loading

kaizencc Dec 31, 2024

iliapolo Jan 1, 2025

kaizencc Jan 2, 2025

iliapolo Jan 2, 2025

kaizencc Jan 2, 2025

aws-cdk-automation commented Jan 2, 2025

mergify bot commented Jan 2, 2025

mergify bot commented Jan 2, 2025

github-actions bot commented Jan 2, 2025

chore(cli): function to turn yargs output to cliArguments #32696

chore(cli): function to turn yargs output to cliArguments #32696

Conversation

kaizencc commented Dec 31, 2024 • edited Loading

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 31, 2024 • edited Loading

Codecov Report

kaizencc Dec 31, 2024

Choose a reason for hiding this comment

iliapolo Jan 1, 2025

Choose a reason for hiding this comment

kaizencc Jan 2, 2025

Choose a reason for hiding this comment

iliapolo Jan 2, 2025

Choose a reason for hiding this comment

kaizencc Jan 2, 2025

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 2, 2025

AWS CodeBuild CI Report

mergify bot commented Jan 2, 2025

mergify bot commented Jan 2, 2025

github-actions bot commented Jan 2, 2025

kaizencc commented Dec 31, 2024 •

edited

Loading

codecov bot commented Dec 31, 2024 •

edited

Loading