docs(iam): hints at accountId as string type #22149
Conversation
|
|
misterjacko
changed the title
misterjacko
changed the title
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Hello, Thank you for taking the time and submitting a review. As per your description:
You added two characters in a file's comment section. Did not refactor any code, did not update any README. While we appreciate the intent, this is not the kind of change that would make a dent in the user experience or type visibility. |
|
I would actually argue that it does provide some value because it clarifies that the account id needs to be a string, and not a number (at least in the docs, if not in the type). Let's go ahead and merge this anyway :) |
|
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). |
---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- With the merge of [aws#20292](aws#20292) the accountID type was essentially required to be a string with the exception of an unencoded token. So, while the type of accountId cannot be set to string and must remain any to preserve compatibility, the hint for accountId should at least suggest that a string should be used rather than an int.
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
With the merge of #20292 the accountID type was essentially required to be a string with the exception of an unencoded token. So, while the type of accountId cannot be set to string and must remain any to preserve compatibility, the hint for accountId should at least suggest that a string should be used rather than an int.