Corrected project to conform to PEP8 style. Passed pep8, py27, cover … #15
Conversation
|
so the coverage test failed, but I can confirm that it did pass locally. I am looking at the Travis config right now to see if I can find out what went wrong and get it working. |
…right with them in the repo. I suspect that this is the problem with the builds on coverage, which worked before.
|
and the cleanup of my cleanup did it! All tests are passing :) |
|
|
||
| # Licensed under the Apache License, Version 2.0 (the "License"); |
There was a problem hiding this comment.
For all files, could you move this licensing statement above the imports? Should be the first thing at the top so you don't have to jump over this to see your imports.
|
|
||
| # subcommands used by the program, like 'bdot init' | ||
| sub_cmd = [{ |
There was a problem hiding this comment.
I consider this format less readable than the previous way you had written this. I know it probably seems overly verbose, but at a glance, I could easily see what was a dict, what was a list, etc. Now, I can't.
| return all_subs | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
See https://github.com/commitmas/pybluedot/blob/master/setup.cfg#L24 and study up on the setuptools configuration. In short, none of the Python files we develop will be called directly - that role will be fulfilled by a console script ("binary") generated by setuptools. What you should do is create a "main()" function here so that the current setuptools configuration knows what to grab.
Then, running this is simple:
sudo python setup.py install
bdot patents
| function: "sounds" | ||
| } | ||
| ] | ||
| cmd_arg = [{ |
There was a problem hiding this comment.
Honestly, this approach of having args in a list like this may be more trouble than it's worth. Just a gut feeling - you may consider seeing what @jeorryb did here: #13 on args, and try to gel this approach with that one. It's possible do to it, but you would have to use the same options for each arg, which you may not want.
| @@ -1,5 +1,17 @@ | |||
| from pybluedot.tests import base | |||
There was a problem hiding this comment.
No unit tests for the functions you've created? 😄
|
Hey @gamefiend anything I can do to help? |
|
Not at this moment. I've just been delayed with a crazy problem at work
that has been eating up the majority of my bandwidth. Late nights, etc.
Should be clear in a day or so.
Thanks,
Quinn
…On Thu, 8 Dec 2016 at 17:51 Matt Oswalt ***@***.***> wrote:
Hey @gamefiend <https://github.com/gamefiend> anything I can do to help?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZC6Ps8rR5IW0jjLedbjSYg2vV7U6-Wks5rGInrgaJpZM4LGLSU>
.
|
|
@gamefiend This is a good PR and I would love to be able to merge it and fix the build! Let me know if you have a chance to address some of my comments, and if there's anything I can do to help. |
…tests in toxs.
The one thing I have been unsure of is whether we want the coverage html code included in the project or whether that is meant to be local. Other than that, everything is good to go. Will return to finish the argument parser creation code in the next few days.