Skip to content

Conversation

taylorjdawson
Copy link
Contributor

No description provided.

@taylorjdawson taylorjdawson mentioned this pull request Jun 25, 2018
@AndreMiras
Copy link
Contributor

Great pull request! Do you mind enforcing the linter check in tox for the fix you did?

@taylorjdawson
Copy link
Contributor Author

@AndreMiras You want me to enforce the linter in tox for all files not just the test folder? @w7788129 I am not sure what your intentions are but you're cluttering our feed. @corpetty is it possible to remove the unnecessary comments?

@AndreMiras
Copy link
Contributor

@taylorjdawson yes exactly. The idea is to enforce tox for at least the fix you made. That way nobody will ever break it again.
So when it's too much work, you can add some exceptions in the tox.ini file and as you fix the issue you remove the exception for it. For example I did that recently in kivy/python-for-android@b3d3d45#diff-b91f3d5bd63fcd17221b267e851608e8
In this commit you see I fix more stuff and at the same time I remove the error from the ignore list.

@taylorjdawson
Copy link
Contributor Author

@AndreMiras I didn't create a separate branch for that PR (elementary mistake) so I just made a new branch 'linting' and will submit a pull request with that branch. Do you know if it is possible to create a new pull request but attach this feed to it?

@AndreMiras
Copy link
Contributor

Simply create your new pull request and copy the link here.

@taylorjdawson
Copy link
Contributor Author

Got it. And I just change commands = flake8 tests/ to commands = flake8 py-etherscan-api/ correct?

@AndreMiras
Copy link
Contributor

AndreMiras commented Jul 22, 2018

It should be commands = flake8 tests/ etherscan/ and eventually at some point we may also check examples/ directory. You can always give it a try locally running tox command. I'm on the gitter chat today if you want.

@taylorjdawson
Copy link
Contributor Author

PR moved to: #40

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