-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add an example construct package #7748
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I love this, but to me it looks like this example is trying to be 2 things at the same time.
It's trying to be a superset of explanation of anything you might want to stuff into a construct, as well as a copy/pasteable base.
Consequence of trying to be the 2nd while also being the 1st, is that there is a LOT OF STUFF in the template that will have to be deleted by a knowledgeable user, and it might not be obvious which parts are optional and which parts aren't.
I think a simple or more additive approach might work better...
I don't have a good ready-made solution either way, except maybe looking into some templating tools like https://github.com/cookiecutter/cookiecutter or https://www.npmjs.com/package/cookiecutter or similar.
I understand you don't really want to go for those, and I will ship this to not let perfect be the enemy of good, but maybe we can spend a couple more hours of thinking about it?
packages/@aws-cdk/example-construct-library/lib/example-resource-utils.ts
Outdated
Show resolved
Hide resolved
import { exampleResourceArnComponents } from './example-resource-utils'; | ||
|
||
/** | ||
* The interface that represents the ExampleResource resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fucking AMAZING job on all the explanations... but I wonder if this is the best place for them.
My experience with explanatory-comments-in-templates is that after a while people just start skipping them and leaving them in (instead of deleting them). So my gut says this all shouldn't live here... unfortunately I don't have a better solution.
Best I can come up with is put all the explanation (basically the Construct Library Guide) up on the GitHub wiki, and putting links to the relevant sections into the comment blocks...
?
packages/@aws-cdk/example-construct-library/lib/example-resource.ts
Outdated
Show resolved
Hide resolved
Also wondering if we should separate out the |
packages/@aws-cdk/example-construct-library/lib/example-resource-utils.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/example-construct-library/lib/example-resource.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/example-construct-library/lib/example-resource.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/example-construct-library/test/integ.example-resource.ts
Show resolved
Hide resolved
packages/@aws-cdk/example-construct-library/lib/example-resource.ts
Outdated
Show resolved
Hide resolved
8b96a45
to
aa29b7f
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aa29b7f
to
854138b
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've beaten this horse enough. Let's get this in and tweak it further as we see the need to.
Blocked by a label if there's any more changes you'd like to get in, otherwise let's do this.
This change adds a new package whose sole purpose is to illustrate the many concepts and patterns used in the CDK Construct Library. The idea is it would serve as the jumping-off point for contributions to the main CDK repo, and also creating independent construct libraries.
854138b
to
2ef1e3e
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Commit Message
feat: add an example construct package
This change adds a new package whose sole purpose is to illustrate the many
concepts and patterns used in the CDK Construct Library.
The idea is it would serve as the jumping-off point for contributions to the main CDK repo,
and also creating independent construct libraries.
End Commit Message
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license