Skip to content

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jun 21, 2019

Legacy print statements are syntax errors in Python 3 but print() function works as expected in both Python 2 and Python 3.

Legacy __print__ statements are syntax errors in Python 3 but __print()__ function works as expected in both Python 2 and Python 3.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 21, 2019
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, I think you can get by without the print future and it's preferred to use single-quote strings / string formatter.

@@ -49,15 +54,15 @@ def _get_plural_forms(js_translations):
for l in js_translations._catalog[''].split('\n'):
if l.startswith('Plural-Forms:'):
plural = l.split(':', 1)[1].strip()
print "plural is %s" % plural
print("plural is %s" % plural)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer string formatter and single quotes:

print(`plural is {}`.format(plural))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python/black (think gofmt for Python) is reformatting code to prefer " over ' but I will make the requested change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll consider this alternative style; I don't personally have a strong opinion either way. Thank you for the pointers, also thanks a ton for the fixes!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding python/black and quotes, according to the style guide it looks like some teams are using yapf.

@gguuss
Copy link
Contributor

gguuss commented Aug 19, 2019

@cclauss Friendly ping, happy to accept your enhancements once you've applied the style we prefer.

Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gguuss gguuss merged commit 02998e8 into GoogleCloudPlatform:master Aug 20, 2019
@cclauss cclauss deleted the modernize-Python-2-codes branch August 20, 2019 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants