-
Notifications
You must be signed in to change notification settings - Fork 267
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
improvement: more useful "create project" yaml #4985
Conversation
873f6d7
to
d5fb0e7
Compare
# This means you don't need any dependencies on your local machine, even the builds can be performed remotely. | ||
# It enables sharing build and test caches with your entire team, which can significantly speed up pipelines and development. | ||
- name: remote | ||
defaultNamespace: garden-remote-\${local.username} |
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.
The backslash seems to end up in the final yaml file. We need to fix that before merging.
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.
Is that true? In theory that should just escape the $
so it doesn't get templated in JS.
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.
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 puzzled about this too. But I ran the command "garden create project" with the dev build, and the backslash was there. 🤔
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.
@TimBeyer I now worked around this mystery and updated the tests.
bb6e30d
to
1a0ffa3
Compare
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 is great! I have a few suggestions around comments, feel free to adept or discard.
# It enables sharing build and test caches with your entire team, which can significantly speed up pipelines and development. | ||
- name: remote | ||
defaultNamespace: garden-remote-${"${local.username}"} | ||
|
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.
# You could also define a staging environment, which is shared by several developers. |
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 decided not to incorporate this one, because it does not add a lot of information– Here I'd value conciseness and usefulness over consistency
1a0ffa3
to
17814b9
Compare
While it is cool that we are re-using the descriptions from our docs, there are some problems with this yaml code: - The comments often do not add any valuable information. For example it's already clear that `name: foobar` is the name of the project. - Another problem is that the description in the reference docs is not necessarily the information a user needs when getting started. This becomes apparent in the comment above `apiVersion`, for example. This commit simplifies the `create project` command implementation to just write a file with hardcoded yaml. The comments are hand-crafted for a good getting-started experience. This commit also updates the docs that refer to the "create project" command. Co-authored-by: Tim <tim@garden.io> Co-authored-by: Anna <anna@garden.io>
17814b9
to
2420c74
Compare
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.
LGTM! Thanks for improving this!
What this PR does / why we need it:
The previous yaml generated by the docs-generator looked like this:
While it is cool that we are re-using the descriptions from our docs,
there are some problems with this yaml code:
it's already clear that
name: foobar
is the name of the project.necessarily the information a user needs when getting started. This
becomes apparent in the comment above
apiVersion
, for example.This commit simplifies the
create project
command implementation tojust write a file with hardcoded yaml. The comments are hand-crafted for
a good getting-started experience.
This commit also updates the docs that refer to the "create project"
command.
The new yaml looks like this:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: