Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emphasize PEP8 (non-TF spacing) compliance in README.md #310

Merged
merged 1 commit into from Jun 23, 2019

Conversation

mels630
Copy link
Contributor

@mels630 mels630 commented Jun 20, 2019

No description provided.

@mels630 mels630 requested a review from a team as a code owner June 20, 2019 16:36
@mels630
Copy link
Contributor Author

mels630 commented Jun 20, 2019

split out from #308

STYLE_GUIDE.md Outdated
@@ -43,3 +43,6 @@ foo._protected_member
#### TensorFlow Conventions

Follow the guidance in the [TensorFlow Style Guide - Conventions](https://www.tensorflow.org/community/contribute/code_style#tensorflow_conventions_and_special_uses).

Please note that Addons follows these conventions but _not_ the entirety of the [TensorFlow Style Guide](https://www.tensorflow.org/community/contribute/code_style).
In particular, as stated above, Python code should conform to [PEP8](https://www.python.org/dev/peps/pep-0008/) _without_ the Tensorflow spacing exception.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this... clarification is certainly needed. Could you modify this slightly to say that we follow "the conventions of the TensorFlow library, but choose to format our code using PEP8 guidelines" or something along those lines?

My first read of this was it was a bit back and forth on which guide to use. Though we format with PEP8 and use the conventions for building upon the library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, makes sense. see if the updated commit (cea8962) reads more cleanly

@mels630 mels630 force-pushed the minor_doc_split/style_guide branch from 7c72d09 to ec33a0b Compare June 20, 2019 20:20
@mels630 mels630 force-pushed the minor_doc_split/style_guide branch from ec33a0b to cea8962 Compare June 20, 2019 20:22
Copy link
Member

@seanpmorgan seanpmorgan left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@seanpmorgan seanpmorgan merged commit e4a1aa0 into tensorflow:master Jun 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants