This repository has been archived by the owner on Dec 31, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
docs(samples): Adding template samples #136
docs(samples): Adding template samples #136
Changes from 18 commits
4e46538
0486123
35be7ec
d2b43a1
d027304
ee4d8ae
41512b9
758069e
83c1a1b
3896df2
a41ee95
deffb61
7301bbd
fe3d984
5d67138
80bb1d8
889b4aa
c6993cf
702ea0e
1e809cc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.