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

Added basic PEP 8 to .travis.yml and fixed currently reported issues. Fixes #147. #148

Merged
merged 9 commits into from Apr 15, 2015

Conversation

mertemba
Copy link
Contributor

Python and Javascript code is checked for trailing whitespaces and blank lines, unused imports and tab indentation.

Fixes #147

Checklist

  • There are no merge conflicts.
  • If there is a related issue, a reference to that issue is in the
    commit message.
  • The PR passes tests both locally (run nosetests) and on Travis CI.
  • The code follows PEP8 guidelines. (use a linter, e.g.
    pylint, to check your code)
  • The new feature is documented in the Release
    Notes
    .
  • The code is backwards compatible. (All public methods/classes must
    follow deprecation cycles.)
  • All reviewer comments have been addressed.

@@ -6,7 +6,6 @@
from numpy.testing import assert_allclose

# local
from ..shapes import *
Copy link
Member

Choose a reason for hiding this comment

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

This is probably incorrectly removed.

@moorepants
Copy link
Member

These are the only JS files that we wrote: https://github.com/pydy/pydy/tree/master/pydy/viz/static/js/dyviz

The ones in external for example of just libraries that we download and we should lint or modify them.

@@ -1,5 +1,4 @@
from sympy import symbols
from sympy.physics.mechanics import *
Copy link
Member

Choose a reason for hiding this comment

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

Likely incorrect.

@moorepants
Copy link
Member

@jellysheep I've add a checklist to the top. Please review those items. Some may not be relevant to this PR because you aren't adding code. But once you complete those items, the reviewers will check them off.

@mertemba
Copy link
Contributor Author

@moorepants Thanks.
However I am sorry, I do not have the time to finish this fix/pull request before the deadline tomorrow, as I am leaving in two hours and will be without access to my computer for two weeks (in spite of the fact that this is easily doable for me with a bit more time, but regarding the original issue title I did not assume that fixing syntax in the whole code was required).

@moorepants
Copy link
Member

@jellysheep Ok, do the best you can on your application and do as much work as you can here before you leave. Good luck!

@mertemba
Copy link
Contributor Author

@moorepants Sorry for the delay, I returned on sunday but was struck down with a cold for a few days.

The removed (and the now checked) whitespaces and imports should be fine now, just leave a comment if something is missing.

I watched the SymPy discussion about formatting PRs, but I think the above comments are ok as they are about PEP8 compliance of the whole codebase, not just minor cosmetic fixes.
If the basic content of this pull request is needed for SymPy as well, I could imagine to integrate this kind of linting there, too.

@moorepants
Copy link
Member

Looks good. Thanks for the contribution.

moorepants added a commit that referenced this pull request Apr 15, 2015
Added basic PEP 8 to .travis.yml and fixed currently reported issues. Fixes #147.
@moorepants moorepants merged commit a503bf1 into pydy:master Apr 15, 2015
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.

Add pylint checks to Travis
2 participants