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-lib): experimental support for programatic CLI api #22790

Closed
wants to merge 1 commit into from

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented Nov 4, 2022


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

@gitpod-io
Copy link

gitpod-io bot commented Nov 4, 2022

@github-actions github-actions bot added the p2 label Nov 4, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team November 4, 2022 19:59
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 4, 2022
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

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.

@mrgrain mrgrain force-pushed the feat/aws-cdk-api branch 2 times, most recently from d6db740 to 4e04c25 Compare November 7, 2022 18:29
@mrgrain mrgrain added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Nov 8, 2022
@@ -0,0 +1,226 @@
import * as core from '@aws-cdk/core';
import { exec as runCli } from 'aws-cdk/lib';
Copy link
Contributor

Choose a reason for hiding this comment

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

I trust that this works in the working copy... I don't fully trust that this works after publishing to NPM, based on trickery and #$%*ery we do to the CLI build before we publish.

Does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does work. This minimal example can be run in a shell:

npm install aws-cdk 
node -e "const { cli } = require('aws-cdk/lib'); cli()"

cli is the existing export. I have no reason to believe exec would behave any different. However it's tricky to proof with the current build system.

Maybe doing the code swap first would be better?

Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

Should we change the package name to something more along the lines of cdk-cli-lib ?

(not strictly that, but something in that vein?)

Ultimately, I'd think the goal would be for all code to live here and the dependency arrow to be reversed, right?

@mrgrain
Copy link
Contributor Author

mrgrain commented Nov 8, 2022

Should we change the package name to something more along the lines of cdk-cli-lib
(not strictly that, but something in that vein?)

Yes, I wanted to ask for feedback here. Do we have rules for when we use the @aws-cdk prefix and when not?
I could see @aws-cdk/cli-lib

Ultimately, I'd think the goal would be for all code to live here and the dependency arrow to be reversed, right?

That's 100% right.

@mrgrain mrgrain changed the title feat(aws-cdk-api): experimental programatic api for aws-cdk feat(cli-lib): experimental support for programatic CLI api Nov 8, 2022
@aws-cdk-automation
Copy link
Collaborator

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/22790/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 96c3029
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mrgrain
Copy link
Contributor Author

mrgrain commented Nov 8, 2022

Closed in favor of #22836

@mrgrain mrgrain closed this Nov 8, 2022
@mrgrain mrgrain deleted the feat/aws-cdk-api branch July 26, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants