docs(appmesh): comprehensive App Mesh readme review#15877
docs(appmesh): comprehensive App Mesh readme review#15877mergify[bot] merged 2 commits intoaws:masterfrom
Conversation
|
|
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
|
Looks good to me. I don't see anything obviously wrong. Good to get it updated with all the final API changes |
|
@dfezzie Thanks for reviewing the PR! |
madeline-k
left a comment
There was a problem hiding this comment.
Looks great! Just one comment about using module qualifiers in code snippets.
| ```ts | ||
| const mesh = new Mesh(stack, 'AppMesh', { | ||
| meshName: 'myAwsmMesh', | ||
| const mesh = new appmesh.Mesh(stack, 'AppMesh', { |
There was a problem hiding this comment.
Remove the appmesh qualifier here and everywhere in the README. Types from the documented module should be unqualified. To make that compile, add a default fixture in this module that imports all the appmesh types that are needed.
Details on Rosetta fixtures: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md#rosetta
An example module-level default fixture: https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-kinesisfirehose/rosetta/default.ts-fixture
There was a problem hiding this comment.
Thanks for attaching a link to the relevant to document!
Pull request has been modified.
madeline-k
left a comment
There was a problem hiding this comment.
The README looks good!
The PR build looks to be failing due to some transient failure in another module. A re-run will hopefully resolve this. After that, you might need to add a rosetta fixture for the build to succeed. To test code snippet compilation locally, you can run: yarn rosetta:extract.
|
Thanks @madeline-k ! Looks like there is no auto-merge and auto-rebuild happening. |
Pull request has been modified.
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). |
Conducting comprehensive review on `README` file. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Conducting comprehensive review on `README` file. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Conducting comprehensive review on
READMEfile.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license