Skip to content
This repository has been archived by the owner on May 28, 2022. It is now read-only.

Add proper PEP8 and linting checks #419

Closed
wants to merge 21 commits into from

Conversation

mtomwing
Copy link
Member

Instead of using pep8, I've opted to use flake8 as it does both PEP8 validation and linting. In addition, anyone else using flake8 in some form during development (like via syntastic in vim) will automatically use our config.

@mtomwing
Copy link
Member Author

TODO: Make sure database.py is changed from DOS to UNIX line endings. It's out of this PR's scope, so I won't do it here.

@mtomwing
Copy link
Member Author

I also caught a syntax error here. If only there were tests to cover this code.

This will be caught by flake8 from now on.

@mtomwing
Copy link
Member Author

Another syntax error here. log was not defined.

@mtomwing
Copy link
Member Author

Please explain why all those QtWidgetItem calls are required around here.

@mtomwing
Copy link
Member Author

mtomwing commented Jan 7, 2014

My removal of the seemingly "unused" resource imports seems to have broken parts of the GUI. I didn't notice this because I mainly test via the unit tests.

@zxiiro
Copy link
Member

zxiiro commented Jan 7, 2014

It's not unused. It's how Qt gets things like images and media. It seems unused because the way Qt works is that you only import it and have access to the binary data inside of it via strings like ":/freeseer/blah.png". It's not like a traditional import where you have to instantiate / call a variable method.

@mtomwing
Copy link
Member Author

mtomwing commented Jan 7, 2014

I think we should explicitly call the resource setup stuff instead of doing it on import. Maybe either in the main app constructors or in the launching scripts?

@zxiiro
Copy link
Member

zxiiro commented Jan 7, 2014

You mean resource.py? Unfortunately we can't have it be done as a single import. Every single file that needs access to it needs to import it separately. That's just how PyQt bindings are setup.

@mtomwing
Copy link
Member Author

mtomwing commented Jan 7, 2014

Isn't it already being done in a single import? AFAIK Python should only be importing resource.py exactly once and therefore only calls qInitResources() once?

def qInitResources():
    QtCore.qRegisterResourceData(0x01, qt_resource_struct, qt_resource_name, qt_resource_data)

def qCleanupResources():
    QtCore.qUnregisterResourceData(0x01, qt_resource_struct, qt_resource_name, qt_resource_data)

qInitResources()

@mtomwing
Copy link
Member Author

Closed in favour of using a gerrit review, and to avoid merge conflict hell.

@mtomwing mtomwing closed this Jan 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants