feat(toolkit-lib): add a return type for toolkit.diff() #368
Conversation
kaizencc
had a problem deploying
to
integ-approval
GitHub Actions
Failure
| expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({ | ||
| action: 'diff', | ||
| level: 'warn', | ||
| code: 'CDK_TOOLKIT_W0000', |
There was a problem hiding this comment.
This is the broadest warning code, should be consider one more specific to CDK Diff?
CDK_TOOLKIT_W4xx?
There was a problem hiding this comment.
Momo specifically wanted this lol. The new directive is default warning code unless its returning data
| @@ -184,11 +192,12 @@ export class DiffFormatter { | |||
|
|
|||
| private formatStackDiffHelper( | |||
| oldTemplate: any, | |||
| stackName: string | undefined, | |||
| stackName: string, | |||
There was a problem hiding this comment.
What if it's an empty string?
There was a problem hiding this comment.
it shouldn't be. but even if it is, i don't think my code would fail. obviously right now if you somehow call formatStackDiff() or formatSecurityDiff on the same stack in the same DiffFormatter, we'd write over the previous diff object.
- we are not calling
formatXxxmultiple times per CLI / toolkit call, so I don't expect this to happen in practice at all. - even if we did overwrite, we'd be writing the same object as the
fullDiffis the same.
| */ | ||
| readonly stackName?: string; | ||
| readonly stackName: string; |
There was a problem hiding this comment.
Breaking change? Not sure if it matters, though.
There was a problem hiding this comment.
diff-formatter isn't public so this isn't breaking. i made this change because we do always have the stackName, since we have the stack
kaizencc
had a problem deploying
to
integ-approval
GitHub Actions
Failure
| stacks: { strategy: StackSelectionStrategy.ALL_STACKS }, | ||
| }); | ||
|
|
||
| console.log(JSON.stringify(result)); |
There was a problem hiding this comment.
Build is failing due to this
kaizencc
had a problem deploying
to
integ-approval
GitHub Actions
Failure
kaizencc
had a problem deploying
to
integ-approval
GitHub Actions
Failure
| constructor(props: DiffFormatterProps) { | ||
| this.ioHelper = props.ioHelper; | ||
| this.oldTemplate = props.templateInfo.oldTemplate; | ||
| this.newTemplate = props.templateInfo.newTemplate; | ||
| this.stackName = props.templateInfo.stackName; | ||
| this.stackName = props.templateInfo.newTemplate.stackName; |
There was a problem hiding this comment.
we are now always taking the stack name from the new template. this always exists. this is what we were doing in practice previously, we are now codifying that and removing stackName as an option
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #368 +/- ##
==========================================
- Coverage 85.61% 80.01% -5.60%
==========================================
Files 219 65 -154
Lines 24250 6885 -17365
Branches 2588 790 -1798
==========================================
- Hits 20761 5509 -15252
+ Misses 3440 1354 -2086
+ Partials 49 22 -27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
toolkit.diff()now returns aTemplateDifffor each stack it diffs.Closes aws/aws-cdk#33182
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license