-
Notifications
You must be signed in to change notification settings - Fork 25
docs(samples): Adding template samples #136
Conversation
from google.cloud import compute_v1 | ||
|
||
|
||
def get_instance_template(project_id: str, template_name: str) -> compute_v1.InstanceTemplate: |
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.
generally the preferred style for newly written samples is one sample per file, as it allows for imports to be included in the region tags: https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/AUTHORING_GUIDE.md#region-tags
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.
Well, the guide you linked doesn't really state that there should be 1:1 ratio between files and samples and I've been doing multiple samples per file for some time now in this repo. See for example vm creation samples. This way imports are included in the samples just fine.
I've read the internal "Code Snippets 201" guide and figured that this is the best way to group the samples. I plan to reorganize the samples soon, to introduce more structure with folders for example. If there are reasons to introduce 1 sample = 1 file division, I'd like to know it.
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 reason is essentially enforcement/ease of use. With your samples, their is only one import so it's very complicated. However if a new import would be added for whatever reason, it should only apply to the sample using it, which becomes more complicated. Same with when an import needs to be removed - if it's no longer using it in the sample (but is in a different sample in the same file), flake8 can't catch that it's only being used in some region tags but not all.
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.
OK, I see your point. I will reorganize those samples soon, as we were planning to do a bigger refactoring of how we keep the samples for the Compute documentation in multiple languages.
Could you just suggest what would be the best course of action, when there are rather big pieces of code repeating in multiple samples like in the vm creation samples? There, the create_with_disks
method is used by 6 different samples. Can a region snap multiple files? If not, I will have to replicate this code multiple times...
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.
It depends on how you arrange the samples on the page. If there really is a certain amount of boiler plate, you could create a different region tag for that section and have a different region tag that references. However, it's probably feasible to just integrate it as part of the main samples.
Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
…ute into samples-templates
Here is the summary of changes. You are about to add 13 region tags.
This comment is generated by snippet-bot.
|
c2c6df2
to
c4a3e00
Compare
c4a3e00
to
80bb1d8
Compare
@kurtisvg Please could you review/approve? |
from google.cloud import compute_v1 | ||
|
||
|
||
def get_instance_template(project_id: str, template_name: str) -> compute_v1.InstanceTemplate: |
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.
It depends on how you arrange the samples on the page. If there really is a certain amount of boiler plate, you could create a different region tag for that section and have a different region tag that references. However, it's probably feasible to just integrate it as part of the main samples.
@kurtisvg, sorry for the ping. I was having trouble with the request review button for @Xmasreturns. It's not working when I click it. |
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 worries 👍
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕