Update Translate samples to establish canonical template#1734
Update Translate samples to establish canonical template#1734
Conversation
alixhami
added
the
do not merge
googlebot
added
the
cla: yes
|
I really, really do not like the idea of our snippets being embedded in tests. Our tests are complex to run. I pushed back on this explicitly when this change was proposed and asked for user research before implementing this. I'm am -1 on the structure, but otherwise fine with the contents. |
| text = 'Hello world' | ||
| target = 'is' # Target must be an ISO 639-1 language code. | ||
| model = translate.NMT | ||
| if isinstance(text, six.binary_type): |
There was a problem hiding this comment.
Use a u prefix on text and it'll always be the Unicode type.
|
|
||
| translate_client = translate.Client() | ||
|
|
||
| text = 'Hello world' |
There was a problem hiding this comment.
I just reviewed another PR that does the same update to a new canonical template, but the placeholder variables looked different there. There was a # TODO(developer) comment. Should that be added above these placeholder variables?
Also, should this be:
text = 'Your text to analyze, e.g. Hello world'
|
|
||
| text = 'Hello world' | ||
| target = 'is' # Target must be an ISO 639-1 language code. | ||
| model = translate.NMT |
There was a problem hiding this comment.
What is NMT? Can this have a comment describing the placeholder variable?
|
Just want to also add my -1 to the format being proposed as well, for the record |
beccasaurus
requested review from
averikitsch
and removed request for
beccasaurus
|
Please address comments or close this PR. Thanks. |
|
Comments have not been addressed in two months. Reopen if needed. |
chore: Add README for running zonal buckets samples
5 participants
Follows new code sample rubric. Please review for compliance with the new code sample rubric and express any language-specific considerations that need to be addressed for this to be the canonical sample.