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

Cleaning up the code? #20

Open
ASmoliak opened this issue Aug 31, 2017 · 5 comments
Open

Cleaning up the code? #20

ASmoliak opened this issue Aug 31, 2017 · 5 comments

Comments

@ASmoliak
Copy link

After making a quick glance at the code I think it can be improved by having it follow the PEP 8 guidelines, really simple things such as docstrings for every module/class/method/function, double blank lines before and after class/function and single blank line for methods,

@abaldwin88
Copy link
Owner

Yep for sure. Please free to open a PR adjusting the pylintrc file and updating the rest of the codebase to bring it in line. I'm good for pretty much all of the suggestions listed in PEP 8. I do prefer the max character length per line at 100 though.

Docstrings are on me though. ( Unless someone else get's really familiar with the code ) I'll write them up when I get a chance but I've got some higher priority things to patch up.

@ASmoliak
Copy link
Author

ASmoliak commented Sep 5, 2017

This is the first time I'm actually trying to contribute on another project on GitHub, I'm using PyCharm, are there some things I should be aware of first?

@abaldwin88
Copy link
Owner

First you'll want to fork this repo and run a local version for development. You should be able to follow the steps here: https://github.com/abaldwin88/roamer/blob/master/CONTRIBUTING.md

I'm not sure if you're familiar with command line linting tools but basically the pylintrc file defines syntax checks. That way anyone who pulls down the repo and uses pylint has the same set of style checks as everyone else regardless of editor. I also have travis ci set to fail the build if pylint raises anything.

My preference is that when making style changes we update the pylintrc file so that pylint throws a warning for the previously written code.

It's been awhile since I've used pycharm but should be able to get pylint working inside it:
https://stackoverflow.com/questions/38134086/how-to-run-pylint-with-pycharm

Don't hesitate to ask questions or submit a half finished PR for feedback.

@ASmoliak
Copy link
Author

Thank you, I will check it out, when the changes are made I simply make a pull request, correct? (after running the unit tests and making sure there's no errors thrown)

@abaldwin88
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants