-
Notifications
You must be signed in to change notification settings - Fork 273
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
0.13 docs update: tutorials #4127
Conversation
To initialize the project, we can use a helper command: | ||
To initialize the project use a helper command: |
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.
👍
@@ -35,55 +38,68 @@ providers: | |||
|
|||
We have one environment (`default`) and a single provider. We'll get back to this later. | |||
|
|||
Next, let's create module configs for each of our two modules, starting with `backend`. | |||
[//]: # (TODO: `garden create action` is still under construction, update this section when the command is ready) |
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 could just skip implementing it and have simple configs here.
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.
Agree, the command can wait. I'll change the tutorial to use explicit configuration code snippets for action definitions.
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.
Changed this in 1bc2439
8e90761
to
27f3448
Compare
would you mind updating the target branch, there are 5k+ line changes in this pr currently |
Ah, sorry, my bad :) I've just updated it, thanks 👍 |
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.
some nitty-pickies
description: Frontend service container | ||
type: container | ||
|
||
# This defines an image to be used and refers the 'frontend' Build action defined above |
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 this can be removed as it's explained above
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.
Removed in e1230b7
dependencies: | ||
- deploy.backend | ||
|
||
# This block is necessary to deploy and expose the frontend application |
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 this can be removed as it's explained above
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.
Removed in e1230b7
spec: | ||
args: [ npm, test ] | ||
|
||
--- # the yaml separator is necessary to delimit different actions |
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 need to repeat the comment imo, just distracts from other more important ones
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.
Removed in e1230b7
Makes editing in IDE easier, no need to horizontally scroll the text.
Co-authored-by: Orzelius <[email protected]>
Use explicit configuration code snippets instead of references to a non-existing command.
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
Co-authored-by: Orzelius <[email protected]>
ec794f3
to
e1230b7
Compare
Thanks for the comments and improvement @Orzelius! Now everything has been addressed and applied. Please review again :) |
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.
All good from me! 🎸
What this PR does / why we need it:
Addresses the tutorials part of #4067.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: