Skip to content

Conversation

@kaizencc
Copy link
Contributor

Related to #32473. This PR fixes it, but is not targeting main.

This PR is meant to pull out a specific feature that still depends on #32760. We are adding include and exclude options to deploy and watch. watch.include/exclude matches 1-1 with what is expected out of cdk.json today, so those properties end up merged together in the same Settings object. There are some legacy considerations, like cdk.json allows a single string instead of a string array, but that does not make sense for the CLI as those are automatically converted to arrays.

Checklist


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

@kaizencc kaizencc requested a review from a team as a code owner January 13, 2025 22:23
@aws-cdk-automation aws-cdk-automation requested a review from a team January 13, 2025 22:23
@github-actions github-actions bot added the p2 label Jan 13, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 13, 2025
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.

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

@aws-cdk-automation aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Jan 13, 2025
*
* @default 'cdk.out'
*/
readonly output?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure we can just get that from the assembly itself.

@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/32903/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.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

1 similar comment
@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/32903/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.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

@paulhcsun
Copy link
Contributor

paulhcsun commented Apr 7, 2025

Closing this as there are no current plans to work on this. The change is still needed and will be continued in a new PR.

NOTE: Please do not manually delete this branch.

@paulhcsun paulhcsun closed this Apr 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2 pr/needs-cli-test-run This PR needs CLI tests run against it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants