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

apply pylint and add to CI #25

Closed
zramsay opened this issue Oct 10, 2017 · 13 comments
Closed

apply pylint and add to CI #25

zramsay opened this issue Oct 10, 2017 · 13 comments
Assignees

Comments

@zramsay
Copy link
Collaborator

zramsay commented Oct 10, 2017

No description provided.

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 10, 2017

yes 100%

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 10, 2017

hey @zramsay, you going to be just doing review or will you be doing some dev too?

blockchain is our defacto branch until issue 26 is done. Assign yourself to an issue if you are going to pick it up.

Thanks for the issues/review :)

@zramsay
Copy link
Collaborator Author

zramsay commented Oct 10, 2017

haha yeah i discovered the "correct" branch. We should use master/develop once its merged. I'll be doing "light" dev work (e.g. tackling linting/useability/documentation, etc)

@zramsay zramsay self-assigned this Oct 10, 2017
@djrtwo
Copy link
Collaborator

djrtwo commented Oct 10, 2017

Cool! Agreed on master/develop.

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 10, 2017

I'll be focussing on organization and refactors for the time being. Probably one level deeper than you.

@naterush Will continue pushing the experimental dev -- oracles, simulations, etc.

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 13, 2017

Check out flake8. does lint+

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 13, 2017

I'm in favor of changing pylint settings to allow for line lengths up to 100 char instead of default 80. I work on a few python projects (web3.py, pyevm) that do this. Saves me a bit of headache and looks good on most screens.

Thoughts?

@karlfloersch
Copy link
Member

karlfloersch commented Oct 13, 2017

@djrtwo I think it's totally reasonable to push the char limit up. The 80 char limit is too restrictive now that everyone has retina displays

@zramsay
Copy link
Collaborator Author

zramsay commented Oct 13, 2017

it's 100 by default already, at least when I run pylint.

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 13, 2017 via email

@zramsay zramsay mentioned this issue Oct 13, 2017
@zramsay zramsay changed the title apply pylint apply pylint and add to CI Oct 13, 2017
@zramsay
Copy link
Collaborator Author

zramsay commented Oct 15, 2017

should we also pylint tests ?

@djrtwo
Copy link
Collaborator

djrtwo commented Oct 15, 2017 via email

@naterush
Copy link
Collaborator

naterush commented Nov 1, 2017

Closing. Will open new issue about getting CI working again.

@naterush naterush closed this as completed Nov 1, 2017
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

No branches or pull requests

4 participants