Skip to content

fix(toolkit-lib): join dynamic values with spaces instead of commas#1219

Open
snese wants to merge 1 commit intoaws:mainfrom
snese:fix/issue-31963-bootstrap-env-separator
Open

fix(toolkit-lib): join dynamic values with spaces instead of commas#1219
snese wants to merge 1 commit intoaws:mainfrom
snese:fix/issue-31963-bootstrap-env-separator

Conversation

@snese
Copy link

@snese snese commented Mar 11, 2026

Issue

Closes aws/aws-cdk#31963

Reason for this change

The addDynamicValues method in NoticesFilter joins multiple dynamic values with commas, producing invalid cdk bootstrap commands like:

cdk bootstrap aws://ACCOUNT/us-east-1,aws://ACCOUNT/us-west-2

Running this command fails with:

Expected environment name in format 'aws://<account>/<region>'

Description of changes

Changed the join separator from , to (space) in addDynamicValues() so the suggested command is valid:

cdk bootstrap aws://ACCOUNT/us-east-1 aws://ACCOUNT/us-west-2

Also updated two JSDoc comments that referenced the old comma separator.

Description of how you validated changes

  • All 55 existing unit tests in notices.test.ts pass
  • Updated the test assertion that verified comma-separated output to expect space-separated output
  • ESLint and TypeScript compilation clean

Checklist

  • My code adheres to the CONTRIBUTING GUIDE
  • If my change affects existing tests, I have updated those tests accordingly

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

The `addDynamicValues` method joined multiple dynamic values with commas,
producing invalid `cdk bootstrap` commands like:
  cdk bootstrap aws://acct/us-east-1,aws://acct/us-west-2

Changed to space separator so the command is valid:
  cdk bootstrap aws://acct/us-east-1 aws://acct/us-west-2

Also updated JSDoc comments to reflect the new separator.

Closes aws/aws-cdk#31963
@github-actions github-actions bot added the p2 label Mar 11, 2026
@aws-cdk-automation aws-cdk-automation requested a review from a team March 11, 2026 17:30
@snese snese changed the title fix(notices): join dynamic values with spaces instead of commas fix(toolkit-lib): join dynamic values with spaces instead of commas Mar 11, 2026
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.79%. Comparing base (69089a7) to head (d50de47).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1219      +/-   ##
==========================================
- Coverage   87.87%   87.79%   -0.08%     
==========================================
  Files          73       73              
  Lines       10168    10168              
  Branches     1348     1343       -5     
==========================================
- Hits         8935     8927       -8     
- Misses       1209     1217       +8     
  Partials       24       24              
Flag Coverage Δ
suite.unit 87.79% <ø> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: Typo and bad command guidance for rebootstrap

2 participants