-
Notifications
You must be signed in to change notification settings - Fork 440
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): What is CDKTF and Interoperability #1065
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.
This looks great - these improvements are great. 👍 pending a few small suggestions
@danieldreier I didn't mean to push the commands page to this branch but alas, I have. So....you can review that too. I'm sorry about that, but then I guess we can check it off our list! |
|
||
The `watch` command watches a directory for changes and automatically synthesizes and deploys changes as they happen. It allows for rapid iterations when developing infrastructure, especially when working with serverless services. It currently supports only one stack at a time. | ||
The `watch` command watches a directory for changes and automatically synthesizes and deploys changes as they happen. It allows for rapid iterations when developing infrastructure, especially when working with serverless services. It currently supports only one stack at a time and automatically deploys changes without asking for confirmation. |
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.
just curious @danieldreier but is there any flag for using watch
that will prevent it from deploying changes without asking for permission? That seems like something maybe worth calling out with a warning?
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.
yeah you know what this is on me - I combined this in with the text and now users may miss it.... I'm going to try combining it with the warning we already have above since we probably want to avoid a bunch of warning blocks. But @danieldreier let us know about the flag as well - if there is one, I can add that too!
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.
Thank you @laurapacilio! I added a few comments throughout, but I think these pages are looking great overall.
Co-authored-by: Sarah Hersh <schersh@users.noreply.github.com>
Co-authored-by: Sarah Hersh <schersh@users.noreply.github.com>
Co-authored-by: Sarah Hersh <schersh@users.noreply.github.com>
Okay @schersh and @danieldreier I pushed up changes that I think address all of Sarah's comments (thank you again!). Let me know if you see anything else. |
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 approving with two comments/caveats:
- Need to verify the file location of the terraformplatform.png
- Still a "TODO" item listed for the
watch
command.
Thanks!
@danieldreier Let us know what you think about the |
|
||
An exemplary invocation of watch could be: | ||
TODO please provide an explanation of what these flags do! | ||
|
||
``` | ||
cdktf watch --stack dev --auto-approve | ||
``` |
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 --stack
is just a positional argument according to the help output. auto-approve
behaves similar to TF Core and will apply changes without additional confirmation. auto-approve
is mandatory right now.
cdktf watch [stack] [OPTIONS]
[experimental] Watch for file changes and automatically trigger a deploy
Positionals:
stack Deploy stack which matches the given id only. Required when more than one stack is present in the app [string]
Options:
--version Show version number [boolean]
--disable-logging Dont write log files. Supported using the env CDKTF_DISABLE_LOGGING. [boolean] [default: true]
--disable-plugin-cache-env Dont set TF_PLUGIN_CACHE_DIR automatically. This is useful when the plugin cache is configured differently. Supported using the env CDKTF_DISABLE_PLUGIN_CACHE_ENV. [boolean] [default: false]
--log-level Which log level should be written. Only supported via setting the env CDKTF_LOG_LEVEL [string]
-a, --app Command to use in order to execute cdktf app [required]
-o, --output Output directory [required] [default: "cdktf.out"]
--auto-approve Auto approve [boolean] [default: false]
-h, --help Show help
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 in this help text, added in an explanation for the example, and fussed with the formatting a bit to make it match the other sections on the page. I'm gonna push and we can make any final tweaks in the pass I want to make before Hashiconf :-)
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. |
In this PR I:
Thank you :-)