-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(cli): 'deploy' and 'diff' silently does nothing when given unknown stack name #16073
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Provisional approval, if you do a small refactor.
Also, about the subject line of the PR: this will go into the CHANGELOG, so think about how a customer reading the CHANGELOG will think about this change.
I would classify it as a bugfix, and if it's a bugfix we generally describe the issue that we fixed. So I would name it something like:
fix(cli): silently does nothing when given unknown stack name
Or something to that effect.
@@ -435,7 +435,11 @@ export class CdkToolkit { | |||
/** | |||
* Validate the stacks for errors and warnings according to the CLI's current settings | |||
*/ | |||
private async validateStacks(stacks: StackCollection) { | |||
private async validateStacks(stacks: StackCollection, stackNames: string[]) { | |||
if (stacks.stackCount == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this check, but I think it should not be part of this function. Validating that there are stacks is not what this function is doing (it's checking stack metadata).
How about adding a new function?
function validateStacksSelected(stacks: StackCollection, stackNames: string[]);
Or something?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…n stack name (aws#16073) Currently, `cdk deploy` and `cdk diff` on stacks that do not exist return no output on the command line. This PR introduces a descriptive error message for those cases so it is easier to understand what happened. Leaving the idea of fuzzy matching for stack name suggestions to a (potential) future PR, I can open a new issue for that if this one goes through. For the linter, I'm not sure if anything of value can be added to the readme so I'm wondering if this would be a candidate for exemption. closes aws#15866 while adding the same idea to `diff` as well as `deploy`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…n stack name (aws#16073) Currently, `cdk deploy` and `cdk diff` on stacks that do not exist return no output on the command line. This PR introduces a descriptive error message for those cases so it is easier to understand what happened. Leaving the idea of fuzzy matching for stack name suggestions to a (potential) future PR, I can open a new issue for that if this one goes through. For the linter, I'm not sure if anything of value can be added to the readme so I'm wondering if this would be a candidate for exemption. closes aws#15866 while adding the same idea to `diff` as well as `deploy`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…n stack name (aws#16073) Currently, `cdk deploy` and `cdk diff` on stacks that do not exist return no output on the command line. This PR introduces a descriptive error message for those cases so it is easier to understand what happened. Leaving the idea of fuzzy matching for stack name suggestions to a (potential) future PR, I can open a new issue for that if this one goes through. For the linter, I'm not sure if anything of value can be added to the readme so I'm wondering if this would be a candidate for exemption. closes aws#15866 while adding the same idea to `diff` as well as `deploy`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…n stack name (aws#16073) Currently, `cdk deploy` and `cdk diff` on stacks that do not exist return no output on the command line. This PR introduces a descriptive error message for those cases so it is easier to understand what happened. Leaving the idea of fuzzy matching for stack name suggestions to a (potential) future PR, I can open a new issue for that if this one goes through. For the linter, I'm not sure if anything of value can be added to the readme so I'm wondering if this would be a candidate for exemption. closes aws#15866 while adding the same idea to `diff` as well as `deploy`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Currently,
cdk deploy
andcdk diff
on stacks that do not exist return no output on the command line. This PR introduces a descriptive error message for those cases so it is easier to understand what happened.Leaving the idea of fuzzy matching for stack name suggestions to a (potential) future PR, I can open a new issue for that if this one goes through.
For the linter, I'm not sure if anything of value can be added to the readme so I'm wondering if this would be a candidate for exemption.
closes #15866 while adding the same idea to
diff
as well asdeploy
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license