Code review comment for lp:~rodsmith/hwcert-tools/get-ytd

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/18/2015 04:11 PM, Zygmunt Krynicki wrote:
> Review: Needs Fixing
>
> Hey.
>
> Thanks for improving this, it looks much better. I must ask you to
> run flake8 though as there are plenty of small things that flake
> will tell you
about.

I'd done that before, but I hadn't had the flake8-docstrings
installed. I've now installed it and it's coming up clean.

A couple more comments below....

>> + if certnum == "all": + request_params = {"username":
>> username, + "api_key": api_key} +
>> else: + request_params = {"username": username, +
>> "api_key": api_key, + "name": certnum}
>
> Don't add empty lines within functions.

Sorry, I must disagree on this one. From a cognitive point of view,
blank lines help separate logically-related sections of functions,
which helps both finding relevant sections of code and understanding
it. In Googling the issue, I found, in PEP-8:

: Use blank lines in functions, sparingly, to indicate logical
: sections.

Judging by comments on online forums, this sentence seems to be
interpreted as anything from "use as many blank lines as you want" to
"don't ever use blank lines." In the face of that ambiguity, I'm
sticking with what both my personal preference and my cognitive
psychology training says works.

>> + "https://certification.canonical.com/me/") +
>> parser.add_argument( + "release", help="Ubuntu release
>> ('trusty', 'precise', etc.)")
>
> don't enumerate this but add choices=('trusty', ...) this will let
> argparse check it automatically

The downside to this is that it will require changes for each new
version. Granted, that's not often, but I'd rather not deal with that
(or, worse, foist it onto somebody who's never seen the code) when
16.04 or 18.04 comes out. As it is now, the script returns a report
with no entries if the code name is mistyped, which IMHO is not a big
deal; and it will work with future releases. Thus, I'd prefer to leave
it as-is.

- --
Rod Smith
Server and Cloud Certification Engineer
<email address hidden>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJVCel3AAoJEFgyRI+V0FjmH4YH/1z1sQGyB5bcE+m2JLTPHt4Y
hZXxJAD81UgRSarVmKwNPgEbtJ+zrk5isMnE+A/e4ZpTisPcZLNjwNA9vyT5jY+k
3AhTv4bZiDQGhXcFrbLZ8mJXYzcbeGo2aboPRljrBs+2SBKuuS6OM02MaKdYQaBs
423F+0pDgZkHHue3DmP/2cMvgTLLRue25QKiP3sncj1ur695Efy2NpSgeZo1j/pd
i5MfbbPY240WsbLgigKJJRWLxBfVhztXPlwPSqnESMMbEyxxNybPL59fYz9KC9+o
pX+4dPbVBTCD7GVO2f4hipfnJWzZqOtH9fltyv5m4A5JJCy4OasxeWwF0xl6Xwg=
=2kQB
-----END PGP SIGNATURE-----