-
Notifications
You must be signed in to change notification settings - Fork 459
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
chore(docs): Laura architecture edits #1137
Conversation
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 added a few comments. In general, I think this is shaping up and I think the diagrams are a good addition. @laurapacilio I think you said you kept some of this content for the future Constructs page as well?
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.
Awesome work, thanks @laurapacilio I like it much better than before.
}); | ||
app.synth(); | ||
``` | ||
Rather than defining resources by hand, you can leverage constructs to reuse existing resource configurations written in your programming language. TODO: Do we have a constructs library that maybe we could link folks to? OR how can we talk about this briefly without going into too much detail. |
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.
No, we don't have construct libraries yet. Perhaps a little example could help, e.g. extracting a couple of resources from a stack in a construct class?
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.
okay I added something - heh, you had an example of a construct I think on your github :-). Is this okay to do? Read what I wrote, let me know if anything is wrong, and let me know if we need to remove the link. I just want to point people toward SOMETHING they can look at until we get our own constructs page up. Once we do that, we can link to those docs on this page.
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.
Left an inline suggestion, other than that 👍
Co-authored-by: Sarah Hersh <[email protected]>
Co-authored-by: Sarah Hersh <[email protected]>
Co-authored-by: Sarah Hersh <[email protected]>
Co-authored-by: Sarah Hersh <[email protected]>
Co-authored-by: Sarah Hersh <[email protected]>
…form-cdk into laura-architecture-edits
okay @schersh and @skorfmann - I addressed all of your comment (I think!) and pushed up a couple more edits. Could both of you just read through the changes and flag anything outstanding that should be fixed? Then, I can address those final bits and we can push this through :-) |
}); | ||
app.synth(); | ||
``` | ||
Here is an example of a [custom construct written in TypeScript](https://github.com/skorfmann/cdktf-hybrid-module/blob/7a84cbea62fbc3c3b7e92c00d75fcaad495cf29b/packages/cdktf-hybrid-module/lib/construct.ts) that creates a machine image. The exported interface allows users to specify the instance type and one or more tags. The rest of the configuration is defined in the construct and is abstracted from the consumer. |
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.
Here is an example of a [custom construct written in TypeScript](https://github.com/skorfmann/cdktf-hybrid-module/blob/7a84cbea62fbc3c3b7e92c00d75fcaad495cf29b/packages/cdktf-hybrid-module/lib/construct.ts) that creates a machine image. The exported interface allows users to specify the instance type and one or more tags. The rest of the configuration is defined in the construct and is abstracted from the consumer. | |
Here is an example of a [custom construct written in TypeScript](https://github.com/skorfmann/cdktf-hybrid-module/blob/7a84cbea62fbc3c3b7e92c00d75fcaad495cf29b/packages/cdktf-hybrid-module/lib/construct.ts) that creates an AWS EC2 instance. The exported interface allows users to specify the instance type and one or more tags. The rest of the configuration is defined in the construct and is abstracted from the consumer. |
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.
Left a few little comments, but overall really good 👍
website/docs/cdktf/index.html.md
Outdated
@@ -36,6 +36,10 @@ CDKTF offers many benefits, but it is not the right choice for every project. Yo | |||
|
|||
You can make this choice for each team and project because CDK for Terraform [interoperates with existing Terraform providers and modules](./concepts/interoperability-workflows.html). | |||
|
|||
## Choosing a Language for your Project | |||
|
|||
To choose which language to use to build CDKTF applications, consider which of the supported languages you are most familiar with, and which language best fits your organization's current tooling. We work towards providing feature parity and a good user experience across all supported languages, but there may be instances when new features will not be available for languages with experimental support. |
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.
we should add the "build my own construct package scenario here". In which case it's definitely Typescript a user should go for, unless they are 100% certain that they really want to have language X only (since jsii
builds will only work based on Typescript projects)
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.
okay! I'm pushing up a change for this now :-)
@laurapacilio I read through the files again, and aside from resolving Sebastian's few comments above, I think this looks great! |
Co-authored-by: Sebastian Korfmann <[email protected]>
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'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Hello! Here's what I have so far for the architecture edits :-)
I did a few things here:
Let me know what you think! I don't think this is done yet, but I would love your thoughts and if we like it, maybe we can merge it into Sebastian's branch and go from there? @schersh I would love your perspective on what's missing right now. I think it might still be missing some meat so let me know what you still think it needs. But I wanted to get something up for you both to look at sooner rather than later!