Update Translate samples to establish canonical template - #1734
Conversation
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| 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.
Sorry, something went wrong.
|
|
||
| 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'
Sorry, something went wrong.
|
|
||
| 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?
Sorry, something went wrong.
|
Just want to also add my -1 to the format being proposed as well, for the record |
Sorry, something went wrong.
|
Please address comments or close this PR. Thanks. |
Sorry, something went wrong.
|
Comments have not been addressed in two months. Reopen if needed. |
Sorry, something went wrong.
chore: Add README for running zonal buckets samples
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.