Code review comment for lp:~javier.collado/utah/static_analysis_7

Revision history for this message
Joe Talbott (joetalbott) wrote :

On Mon, Oct 15, 2012 at 02:12:20PM -0000, Max Brustkern wrote:
> Review: Needs Information
>
> I've got questions! They may reveal gaps in my understanding of proper python programming.
>
> It seems like in some cases, like config.py and client/testsuite.py, docstrings are being changed from
> """
> Do a thing.
> """
> to
> # Do a thing.
> But not everywhere. Is there a reason only some of them are coming up?

I have a bad habit of commenting out large blocks of code by turning
them into strings with """ or '''. Because we're doing in-source
documentation and python convention uses these strings (depending on
where they are located) as documentation. So, in short, comments
should use the comment character '#' and documentation should be in
apparently. I'll leave it to others to argue if comments are
documentation or not. The way I look at it is if your comment is
related to a few lines of code it won't make sense in the on-line
documents.
>
> What's the deal with cls instead of self? Is that something we should be using elsewhere, or is there some reason it only gets changed in the one instance in preseed.py?
>
> How come in provisioning.py we have _stdout and stderr, but in timeout.py we have stdout and _stderr?
>
> Why do a bunch of parameters change to have underscores before them, but not all of them?

I'm used to the convention that a leading underscore means internal use.
Apparently pylint's authors think that prefixing an unused variable with
an underscore is a way to indicate to pylint that we meant to include a
variable that we aren't yet using.

I don't really agree with this and think it makes the code more
confusing.

Joe

« Back to merge proposal