refactor(cli): remove unused internal code and options #33432
Conversation
mrgrain
temporarily deployed
to
test-pipeline
GitHub Actions
Inactive
| /** | ||
| * Options to pass on to `buildAssets()` function | ||
| */ | ||
| readonly buildOptions?: BuildAssetsOptions; |
There was a problem hiding this comment.
Only used by buildSingleAsset. None of the call sites specify buildOptions. Not used in buildSingleAsset at all.
| /** | ||
| * Options to pass on to `publishAsests()` function | ||
| */ | ||
| readonly publishOptions?: Omit<PublishAssetsOptions, 'buildAssets'>; |
There was a problem hiding this comment.
Only used by publishSingleAsset. None of the call sites specify publishOptions . Not used in publishSingleAsset at all.
| return stack.exists; | ||
| } | ||
|
|
||
| private async prepareAndValidateAssets(asset: cxapi.AssetManifestArtifact, options: AssetOptions) { |
There was a problem hiding this comment.
Only used by the removed methods
| export interface BuildAssetsOptions { | ||
| /** | ||
| * Print progress at 'debug' level | ||
| */ | ||
| readonly quiet?: boolean; | ||
|
|
||
| /** | ||
| * Build assets in parallel | ||
| * | ||
| * @default true | ||
| */ | ||
| readonly parallel?: boolean; | ||
| } | ||
|
|
||
| /** | ||
| * Use cdk-assets to build all assets in the given manifest. | ||
| */ | ||
| export async function buildAssets( | ||
| manifest: AssetManifest, | ||
| sdk: SdkProvider, | ||
| targetEnv: Environment, | ||
| options: BuildAssetsOptions = {}, | ||
| ) { | ||
| // This shouldn't really happen (it's a programming error), but we don't have | ||
| // the types here to guide us. Do an runtime validation to be super super sure. | ||
| if ( | ||
| targetEnv.account === undefined || | ||
| targetEnv.account === UNKNOWN_ACCOUNT || | ||
| targetEnv.region === undefined || | ||
| targetEnv.account === UNKNOWN_REGION | ||
| ) { | ||
| throw new ToolkitError(`Asset building requires resolved account and region, got ${JSON.stringify(targetEnv)}`); | ||
| } | ||
|
|
||
| const publisher = new AssetPublishing(manifest, { | ||
| aws: new PublishingAws(sdk, targetEnv), | ||
| progressListener: new PublishingProgressListener(options.quiet ?? false), | ||
| throwOnError: false, | ||
| publishInParallel: options.parallel ?? true, | ||
| buildAssets: true, | ||
| publishAssets: false, | ||
| }); | ||
| await publisher.publish(); | ||
| if (publisher.hasFailures) { | ||
| throw new ToolkitError('Failed to build one or more assets. See the error messages above for more information.'); | ||
| } | ||
| } | ||
|
|
| export interface PublishAssetsOptions { | ||
| /** | ||
| * Print progress at 'debug' level | ||
| */ | ||
| readonly quiet?: boolean; | ||
|
|
||
| /** | ||
| * Whether to build assets before publishing. | ||
| * | ||
| * @default true To remain backward compatible. | ||
| */ | ||
| readonly buildAssets?: boolean; | ||
|
|
There was a problem hiding this comment.
Only a single internal call-site of publishAssets is left and it is not specifying these options.
| const publisher = new AssetPublishing(manifest, { | ||
| aws: new PublishingAws(sdk, targetEnv), | ||
| progressListener: new PublishingProgressListener(options.quiet ?? false), | ||
| progressListener: new PublishingProgressListener(), | ||
| throwOnError: false, | ||
| publishInParallel: options.parallel ?? true, | ||
| buildAssets: options.buildAssets ?? true, | ||
| buildAssets: true, | ||
| publishAssets: true, | ||
| quiet: options.quiet, | ||
| quiet: false, |
There was a problem hiding this comment.
Only a single internal call-site of publishAssets is left and it is not specifying quiet or buildAssets. So we just use the default value.
|
|
||
| class PublishingProgressListener implements IPublishProgressListener { | ||
| constructor(private readonly quiet: boolean) {} | ||
| constructor() {} |
There was a problem hiding this comment.
Removed unused quiet option upstream in favor of it always being the default (false).
|
|
||
| public onPublishEvent(type: EventType, event: IPublishProgress): void { | ||
| const handler = this.quiet && type !== 'fail' ? debug : EVENT_TO_LOGGER[type]; | ||
| const handler = EVENT_TO_LOGGER[type]; |
There was a problem hiding this comment.
Removed unused quiet option upstream in favor of it always being the default (false) => we just use the default part of the conditional.
|
➡️ PR build request submitted to A maintainer must now check the pipeline and add the |
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (50.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #33432 +/- ##
==========================================
+ Coverage 80.90% 81.00% +0.09%
==========================================
Files 238 238
Lines 14287 14269 -18
Branches 2495 2492 -3
==========================================
- Hits 11559 11558 -1
+ Misses 2442 2425 -17
Partials 286 286
Flags with carried forward coverage won't be shown. Click here to find out more.
|
mrgrain
added
pr-linter/exempt-codecov
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
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). |
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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
mergify
bot
deleted the
mrgrain/chore/cli/remove-unused-code-and-options
branch
|
Comments on closed issues and PRs are hard for our team to see. |
3 participants
Reason for this change
Unused code path that was previously deprecated needs to be removed in preparation of refactoring.
Description of changes
Removing previously deprecated and unused methods on
Deploymentsclass and transitively unused code.Removed options for asset building/publishing that are unused.
Describe any new or updated permissions being added
n/a
Description of how you validated changes
existing tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license