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

WIP for #115 : PEP8 refactor, CI to be added #116

Closed
wants to merge 1 commit into from

Conversation

isaac-philip
Copy link

In addition to the pep8 formatting from flake8 errors/warnings,

certain mentions :

  1. added an entry to .gitignore for vscode
  2. One file needed a more logical change
    auditwheel/tools.py on Line No. 47
    with open(glob(pjoin(out_dir, '*.dist-info/RECORD'))[0]) as f:
    The f is not needed.

Git message,
Reformatted python source as per PEP8

Modified all files referencing to pep8 guidelines

Used flake8 for audit.

Used black to format some files

The CI part using tox/travis is yet to be done
and will be taken ahead by maintainers.

Reformatted python source as per PEP8

Modified all files referencing to pep8 guidelines

Used flake8 for audit.

Used black to format some files

The CI part using tox/travis is yet to be done
    and will be taken ahead by maintainers.
@isaac-philip
Copy link
Author

Travis build failing, something I can change in code? Please have a quick look and let me know.

@isaac-philip
Copy link
Author

../../../virtualenv/python3.6.3/lib/python3.6/site-packages/auditwheel/policy/external_references.py:7: in <module> from . import load_policies E ImportError: cannot import name 'load_policies'
Please advise how to proceed with this, there seems to be an import error.
I cant find the last merge failed with this error i.e. Last PR
Please guide.

Copy link
Member

@ehashman ehashman left a comment

Choose a reason for hiding this comment

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

No changes were made to the Travis tests that would cause linting to run in CI or tox, so we will need to include that.

It is difficult to debug the source of the problem because there are so many unrelated quote changes. Normally single quotes are used in individual strings so I'm not sure what linting profile you used that indicated this needed to change, and the way to do automated tests wasn't checked in so I can't reproduce.

I suggest that you revert the quote changes and that might make it easier to track down the problem?

@@ -1,5 +1,5 @@
import sys
from .main import main

if __name__ == '__main__':
if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what docstyle you have applied but I don't want to switch from single quotes to double quotes.

Copy link
Author

Choose a reason for hiding this comment

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

Sure I'll make the changes as you have said and place it in a day or two. thanks.

Copy link
Author

Choose a reason for hiding this comment

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

I believe that was due to the use of the black formatter as per a suggestion in the conversation thread.
Nevertheless I will revert to as per your suggestion.

@ehashman
Copy link
Member

ehashman commented Nov 8, 2018

Closing this in favour of #124, didn't realize you opened a new PR.

@ehashman ehashman closed this Nov 8, 2018
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