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

Automated code format update #142

Merged
merged 2 commits into from
Jun 26, 2018
Merged

Automated code format update #142

merged 2 commits into from
Jun 26, 2018

Conversation

hardbyte
Copy link
Collaborator

@hardbyte hardbyte commented May 4, 2018

Does the manual component of #99.

I just selected all files and asked pycharm to clean em up

@hardbyte hardbyte requested a review from nbgl May 4, 2018 01:47
Copy link
Contributor

@nbgl nbgl left a comment

Choose a reason for hiding this comment

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

It’s mostly good. It seems to bring comment-style type annotations out of alignment though. Is this configurable?

keys, # type: Sequence[bytes]
k, # type: int
l, # type: int
encoding # type: str
Copy link
Contributor

Choose a reason for hiding this comment

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

In Mypy’s Python 2 examples, the type annotations are aligned like in the original.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had a quick look and it didn't appear to be configurable 👎

I'm not particularly keen to go through and manually edit... if you feel strongly about it do you want to make any changes (or look into other tools that might reformat more optimally)

import hmac
import math
import struct
from enum import Enum
from functools import partial
from hashlib import md5, sha1
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that it’s listing namespace imports (import foo) before from imports (from foo import bar). This behaviour doesn’t seem to be part of PEP8. Looks better though!

# with json.decoder.JSONDecodeError,
# but that doesn't exist in Python 2.
# with json.decoder.JSONDecodeError,
# but that doesn't exist in Python 2.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, fair enough. This comment should be on its own line in the first place.

@@ -217,14 +216,14 @@ def get_master_schema(version):

try:
schema_bytes = pkgutil.get_data('clkhash',
'master-schemas/{}'.format(file_name))
'master-schemas/{}'.format(file_name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Good!

@hardbyte hardbyte force-pushed the feature-code-format-update branch from b8b5443 to b2bbe8e Compare May 18, 2018 03:51
@hardbyte
Copy link
Collaborator Author

As this got left behind and we had another PR which addressed code formating I've force pushed. Would you mind taking another quick look @nbgl

Copy link
Contributor

@nbgl nbgl left a comment

Choose a reason for hiding this comment

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

I’m happy with most of the changes.

It slightly bothers me that PyCharm brings comments out of alignment (think comment-styles type annotations), as this is against convention. But it’s not a dealbreaker.

@hardbyte hardbyte merged commit afe37eb into master Jun 26, 2018
@hardbyte hardbyte deleted the feature-code-format-update branch June 26, 2018 04:14
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.

2 participants