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

Pep 8 Style Guide for Python Code #162

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Conversation

bobinson
Copy link
Contributor

This PR brings in PEP-8 compatibility to the Tinman codebase.

@inertia186
Copy link
Contributor

inertia186 commented Oct 19, 2018

I am not the king of the hill. I'm happy to change code styles if they make sense. Maybe you can sell me on this idea that I personally hate.

I like to use this style:

a = {
  "bbb": "b",
  "cc": "c",
  "ddddddd": {
    "eee": "eeeeeeeeee"
  }
}

Other people like this (which I hate):

a = {
      "bbb": "b",
       "cc": "c",
  "ddddddd": {
    "eee": "eeeeeeeeee"
  }
}

The thing I'm demonstrating is that the : is what's used to align the lines, and I don't like it. The reason I don't like it is because if we add a key in this example, it might require changing unrelated lines to maintain the alignment.

@relativityboy
Copy link
Contributor

We do not include IDE specific files in our repos. In addition, before merging something like this we'd need to see if any of our other repos have adopted style-guides for python, and evaluate.

@bobinson - I'd suggest creating an issue, outlining what it is you're trying to achieve with Pep 8 and how it makes tinman better than it is today. That'll give us something to review-from.

I'm leaving this PR open, but it is not to be considered for merging until the .idea files are gone, and the above actions have been taken.

@bobinson
Copy link
Contributor Author

@inertia186 & @relativityboy : I will revert with an issue report detailing why we need a generic coding style. Also removed the non-standard editor files which was there by mistake.

Just to add to @inertia186's points, I am also not "religious" about a certain coding style over another one (for that matter, I am not even religious about Vim or Emacs). For example, in this very PR I had kept the following non PEP8 compliant section as I found it pretty neat ;-)

image

@relativityboy
Copy link
Contributor

We see the changes. Thanks. Doing some internal legwork to see if this is something we want to adopt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants