Re: [KVM-AUTOTEST] Style guides for autotest

Re: [KVM-AUTOTEST] Style guides for autotest

[Michael Goldish]

Sorry, I forgot two things:

- PEP 8 states that class methods should be separated by a single blank line (whereas top level functions should be separated by two blank lines). After the recent modifications, the KVM test no longer follows this convention as its class methods are separated by two blank lines.

- PEP 257 states that a class docstring should be separated by a single blank line from the class header and from the first method following the docstring, e.g.

class VM:

    """
    This class ...
    """

    def __init__(...

The KVM test no longer follows this convention after the merge (in kvm_vm.py, kvm_config.py, kvm_utils.py, kvm.py and possibly other files).

There may be a few more style "violations" or inconsistencies I forgot to mention, or just haven't noticed yet.

I realize PEPs are merely recommendations, but since Autotest doesn't have a consistent style of its own, why not follow the PEPs?

Thanks,
Michael

----- Original Message -----
From: "Michael Goldish" <mgoldish@redhat.com>
To: "Lucas Meneghel Rodrigues" <lmr@redhat.com>
Cc: jasowang@redhat.com, aeromenk@redhat.com, "Mike Burns" <mburns@redhat.com>, yzhou@redhat.com, supriyak@in.ibm.com, byang@redhat.com, "Yogi" <anantyog@linux.vnet.ibm.com>, "David Huff" <dhuff@redhat.com>, "Dor Laor" <dlaor@redhat.com>, "Autotest mailing list" <autotest@test.kernel.org>, "KVM mailing list" <kvm@vger.kernel.org>
Sent: Sunday, June 7, 2009 3:45:00 PM (GMT+0200) Auto-Detected
Subject: Re: [KVM-AUTOTEST] Style guides for autotest

Hi Lucas,

I'm a little confused about docstrings -- maybe you can clarify this for me.
I've looked at the new KVM test that contains your recent modifications, and found a few inconsistencies:

- In the KVM test the summary line of a docstring appears on the line following the opening quotes, e.g.

"""
Summary line.

Description.
"""

but other Autotest source files (e.g. client/common_lib/pxssh.py) use the previous format (from before the recent modifications) where the summary line appears on the same line as the opening quotes:

"""Summary line.

Description.
"""

(Autotest is also somewhat inconsistent with itself -- every file in client/common_lib has its own docstring conventions.)

- In some functions in the KVM test (e.g. create() in kvm_vm.py) the summary line, followed by a blank line, is actually a summary paragraph consisting of several lines. PEP 257 (http://www.python.org/dev/peps/pep-0257/#multi-line-docstrings) states that the summary line should fit on a single line (and should be followed by a blank line).

- In the KVM test the @param/@author/@note keywords are used. In other files in Autotest (e.g. client/common_lib/packages.py) they are not.

- In kvm_config.py, the @param lines are indented relative to the first docstring line. In other files (e.g. kvm_vm.py) they are not.

Thanks,
Michael

----- Original Message -----
From: "Lucas Meneghel Rodrigues" <lmr@redhat.com>
To: "Autotest mailing list" <autotest@test.kernel.org>, "KVM mailing list" <kvm@vger.kernel.org>
Cc: jasowang@redhat.com, aeromenk@redhat.com, "Mike Burns" <mburns@redhat.com>, yzhou@redhat.com, supriyak@in.ibm.com, "Michael Goldish" <mgoldish@redhat.com>, byang@redhat.com, "Yogi" <anantyog@linux.vnet.ibm.com>, "David Huff" <dhuff@redhat.com>, "Dor Laor" <dlaor@redhat.com>
Sent: Friday, June 5, 2009 8:55:01 PM (GMT+0200) Auto-Detected
Subject: [KVM-AUTOTEST] Style guides for autotest

One thing I did forget to mention on the patch queue e-mail is the
reference to the autotest coding standard:

http://autotest.kernel.org/browser/trunk/CODING_STYLE

There's a convention on docstrings that, besides discussed on the
mailing list, wasn't still formally proposed and integrated to this
document. Will do that in a minute :)

So, to the patch authors, please keep in mind the above guide when
re-working your patches.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [KVM-AUTOTEST] Style guides for autotest

[Lucas Meneghel Rodrigues]

On Sun, 2009-06-07 at 09:18 -0400, Michael Goldish wrote:
> Sorry, I forgot two things:
> 
> - PEP 8 states that class methods should be separated by a single blank line (whereas top level functions should be separated by two blank lines). After the recent modifications, the KVM test no longer follows this convention as its class methods are separated by two blank lines.

On autotest, we opted by separate both functions and class methods using
2 lines, 'overriding' the PEP on this aspect in particular.

> - PEP 257 states that a class docstring should be separated by a single blank line from the class header and from the first method following the docstring, e.g.
> 
> class VM:
> 
>     """
>     This class ...
>     """
> 
>     def __init__(...
> 
> The KVM test no longer follows this convention after the merge (in kvm_vm.py, kvm_config.py, kvm_utils.py, kvm.py and possibly other files).

Indeed we didn't follow that recommendation. Will look at fix that one.

> There may be a few more style "violations" or inconsistencies I forgot to mention, or just haven't noticed yet.
> 
> I realize PEPs are merely recommendations, but since Autotest doesn't have a consistent style of its own, why not follow the PEPs?

Agree. On the CODING_STYLE document, we state:

{{{
Base coding style

When writing python code, unless otherwise stated, stick to the python
style guide (http://www.python.org/dev/peps/pep-0008/).
}}}

We want to make the autotest coding standard conventions as consistent
as possible. Inside the code, some mistakes might have happened, and
eventually, those will be fixed out.

Thanks for your comments, will work to get things as consistent as
possible!


-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST] Style guides for autotest

[Lucas Meneghel Rodrigues]

On Sun, 2009-06-07 at 08:45 -0400, Michael Goldish wrote:
> Hi Lucas,

Hi Michael! The docstring standard wasn't defined for quite some time on
autotest, but now we're on our way to have a definition. On January
(2009) we had some conversation about the docstring format but we didn't
formalize the results of the discussion. During the previous week, I've
proposed a patch to the style guide with the format I've used on the kvm
test 

http://test.kernel.org/pipermail/autotest/2009-June/003567.html

So older autotest code will probably not follow this convention, and
eventually we can fix the non compliances with time.

> I'm a little confused about docstrings -- maybe you can clarify this for me.
> I've looked at the new KVM test that contains your recent modifications, and found a few inconsistencies:
> 
> - In the KVM test the summary line of a docstring appears on the line following the opening quotes, e.g.
> 
> """
> Summary line.
> 
> Description.
> """
> 
> but other Autotest source files (e.g. client/common_lib/pxssh.py) use the previous format (from before the recent modifications) where the summary line appears on the same line as the opening quotes:
> 
> """Summary line.
> 
> Description.
> """
> (Autotest is also somewhat inconsistent with itself -- every file in client/common_lib has its own docstring conventions.)
> 
> - In some functions in the KVM test (e.g. create() in kvm_vm.py) the summary line, followed by a blank line, is actually a summary paragraph consisting of several lines. PEP 257 (http://www.python.org/dev/peps/pep-0257/#multi-line-docstrings) states that the summary line should fit on a single line (and should be followed by a blank line).

We are aiming for a format slightly different than the recommendations
on the PEP. @brief would be the summary of the function, if needed.

> - In the KVM test the @param/@author/@note keywords are used. In other files in Autotest (e.g. client/common_lib/packages.py) they are not.

As the docstring format is new, older code won't follow it. Eventually
we might want to make patches to straighten things out, but it's not a
priority.

> - In kvm_config.py, the @param lines are indented relative to the first docstring line. In other files (e.g. kvm_vm.py) they are not.

My mistake :)

> Thanks,
> Michael

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

Re: [KVM-AUTOTEST] Style guides for autotest

[Michael Goldish]

Hi Lucas,

I'm a little confused about docstrings -- maybe you can clarify this for me.
I've looked at the new KVM test that contains your recent modifications, and found a few inconsistencies:

- In the KVM test the summary line of a docstring appears on the line following the opening quotes, e.g.

"""
Summary line.

Description.
"""

but other Autotest source files (e.g. client/common_lib/pxssh.py) use the previous format (from before the recent modifications) where the summary line appears on the same line as the opening quotes:

"""Summary line.

Description.
"""

(Autotest is also somewhat inconsistent with itself -- every file in client/common_lib has its own docstring conventions.)

- In some functions in the KVM test (e.g. create() in kvm_vm.py) the summary line, followed by a blank line, is actually a summary paragraph consisting of several lines. PEP 257 (http://www.python.org/dev/peps/pep-0257/#multi-line-docstrings) states that the summary line should fit on a single line (and should be followed by a blank line).

- In the KVM test the @param/@author/@note keywords are used. In other files in Autotest (e.g. client/common_lib/packages.py) they are not.

- In kvm_config.py, the @param lines are indented relative to the first docstring line. In other files (e.g. kvm_vm.py) they are not.

Thanks,
Michael

----- Original Message -----
From: "Lucas Meneghel Rodrigues" <lmr@redhat.com>
To: "Autotest mailing list" <autotest@test.kernel.org>, "KVM mailing list" <kvm@vger.kernel.org>
Cc: jasowang@redhat.com, aeromenk@redhat.com, "Mike Burns" <mburns@redhat.com>, yzhou@redhat.com, supriyak@in.ibm.com, "Michael Goldish" <mgoldish@redhat.com>, byang@redhat.com, "Yogi" <anantyog@linux.vnet.ibm.com>, "David Huff" <dhuff@redhat.com>, "Dor Laor" <dlaor@redhat.com>
Sent: Friday, June 5, 2009 8:55:01 PM (GMT+0200) Auto-Detected
Subject: [KVM-AUTOTEST] Style guides for autotest

One thing I did forget to mention on the patch queue e-mail is the
reference to the autotest coding standard:

http://autotest.kernel.org/browser/trunk/CODING_STYLE

There's a convention on docstrings that, besides discussed on the
mailing list, wasn't still formally proposed and integrated to this
document. Will do that in a minute :)

So, to the patch authors, please keep in mind the above guide when
re-working your patches.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies

[KVM-AUTOTEST] Style guides for autotest

[Lucas Meneghel Rodrigues]

One thing I did forget to mention on the patch queue e-mail is the
reference to the autotest coding standard:

http://autotest.kernel.org/browser/trunk/CODING_STYLE

There's a convention on docstrings that, besides discussed on the
mailing list, wasn't still formally proposed and integrated to this
document. Will do that in a minute :)

So, to the patch authors, please keep in mind the above guide when
re-working your patches.

-- 
Lucas Meneghel Rodrigues
Software Engineer (QE)
Red Hat - Emerging Technologies