You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2012/03/07 11:27:15 UTC

svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Author: julianfoad
Date: Wed Mar  7 10:27:15 2012
New Revision: 1297921

URL: http://svn.apache.org/viewvc?rev=1297921&view=rev
Log:
Improve doc strings of Sandbox.simple_*() test suite functions.

Modified:
    subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Modified: subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py?rev=1297921&r1=1297920&r2=1297921&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py Wed Mar  7 10:27:15 2012
@@ -210,7 +210,8 @@ class Sandbox:
     svntest.main.run_svn(False, 'update', target)
 
   def simple_switch(self, url, target=None):
-    """Switch a TARGET to URL"""
+    """Switch the WC or TARGET to URL.
+       TARGET is a relpath relative to the WC."""
     if target is None:
       target = self.wc_dir
     else:
@@ -232,61 +233,72 @@ class Sandbox:
                          target)
 
   def simple_rm(self, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Schedule TARGETS for deletion.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'rm', *targets)
 
   def simple_mkdir(self, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Create TARGETS as directories scheduled for addition.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'mkdir', *targets)
 
   def simple_add(self, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Schedule TARGETS for addition.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'add', *targets)
 
   def simple_revert(self, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Revert TARGETS.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'revert', *targets)
 
   def simple_propset(self, name, value, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Set property NAME to VALUE on TARGETS.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'propset', name, value, *targets)
 
   def simple_propdel(self, name, *targets):
-    """TARGET is a relpath relative to the WC."""
+    """Delete property NAME from TARGETS.
+       TARGETS are relpaths relative to the WC."""
     assert len(targets) > 0
     targets = self.ospaths(targets)
     svntest.main.run_svn(False, 'propdel', name, *targets)
 
   def simple_copy(self, source, dest):
-    """SOURCE and DEST are relpaths relative to the WC."""
+    """Copy SOURCE to DEST in the WC.
+       SOURCE and DEST are relpaths relative to the WC."""
     source = self.ospath(source)
     dest = self.ospath(dest)
     svntest.main.run_svn(False, 'copy', source, dest)
 
   def simple_move(self, source, dest):
-    """SOURCE and DEST are relpaths relative to the WC."""
+    """Move SOURCE to DEST in the WC.
+       SOURCE and DEST are relpaths relative to the WC."""
     source = self.ospath(source)
     dest = self.ospath(dest)
     svntest.main.run_svn(False, 'move', source, dest)
 
   def simple_repo_copy(self, source, dest):
-    """SOURCE and DEST are relpaths relative to the repo root."""
+    """Copy SOURCE to DEST in the repository, committing the result with a
+       default log message.
+       SOURCE and DEST are relpaths relative to the repo root."""
     svntest.main.run_svn(False, 'copy', '-m', svntest.main.make_log_msg(),
                          self.repo_url + '/' + source,
                          self.repo_url + '/' + dest)
 
   def simple_append(self, dest, contents, truncate=False):
-    """Append CONTENTS to file DEST, optionally truncating it first."""
+    """Append CONTENTS to file DEST, optionally truncating it first.
+       DEST is a relpath relative to the WC."""
     open(self.ospath(dest), truncate and 'w' or 'a').write(contents)
 
 



Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> Greg Stein <gs...@gmail.com> writes:
>
>> Geez. Be smart, Philip. Obviously, that quoted content is dumb. I'm talking
>> about format. Specifically not following the basic rule of "one line
>> summary. One blank line. Details." Julian's change is missing the blank
>> lines, which many tools parse.
>
> That's what I don't understand.  Are we expecting people to use tools to
> parse these docstrings?  If we have to repeat default parameters in the
> docstring it it something we can get wrong, if we don't repeat them they
> can't be wrong.  What do we gain?

Maybe what you want is that we follow some of PEP-257 but not all of it?
How should the docstring change?

    """Append CONTENTS to file DEST, optionally truncating it first.
       DEST is a relpath relative to the WC."""

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:

>>  Greg Stein <gs...@gmail.com> writes:
>>> I'm  talking about format. Specifically not following the basic
>>> rule of  "one line summary. One blank line. Details." Julian's
>>> change is missing the blank lines, which many tools parse.
>>> 
>>>  That's what I don't understand.  Are we expecting people to use 
>>> tools to  parse these docstrings?  [...]
[...]
> 
> So which bits of PEP-257 do we follow?  [...]

Sounds like Greg's really just asking for a blank line after the summary.  I'm happy to start doing that in future; I think it can be quite a helpful habit (also, or more so, in our C code).

FWIW I wasn't changing doc strings from single-line to multi-line in this commit, I was just adding some missing info in a style consistent with surrounding code AFAIK.

- Julian

Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Philip Martin <ph...@wandisco.com>.
"Bert Huijben" <be...@qqmail.nl> writes:

>> -----Original Message-----
>> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of Philip
>> Martin
>> Sent: woensdag 7 maart 2012 13:42
>> To: Greg Stein
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1297921 -
>> /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
>> 
>> Greg Stein <gs...@gmail.com> writes:
>> 
>> > Geez. Be smart, Philip. Obviously, that quoted content is dumb. I'm
> talking
>> > about format. Specifically not following the basic rule of "one line
>> > summary. One blank line. Details." Julian's change is missing the blank
>> > lines, which many tools parse.
>> 
>> That's what I don't understand.  Are we expecting people to use tools to
>> parse these docstrings?  If we have to repeat default parameters in the
>> docstring it it something we can get wrong, if we don't repeat them they
>> can't be wrong.  What do we gain?
>
> I wouldn't be surprised if some editors (Eclipse? Others) use or will use
> this syntax for tooltips, to help users when editing.

So which bits of PEP-257 do we follow?  Do we stop writing parameters in
CAPITALS?  Do we repeat arguments one per line?  With defaults?  Do we
document the exceptions that can be raised?  Whether "keyword arguments
are part of the interface"?

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

RE: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of Philip
> Martin
> Sent: woensdag 7 maart 2012 13:42
> To: Greg Stein
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1297921 -
> /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py
> 
> Greg Stein <gs...@gmail.com> writes:
> 
> > Geez. Be smart, Philip. Obviously, that quoted content is dumb. I'm
talking
> > about format. Specifically not following the basic rule of "one line
> > summary. One blank line. Details." Julian's change is missing the blank
> > lines, which many tools parse.
> 
> That's what I don't understand.  Are we expecting people to use tools to
> parse these docstrings?  If we have to repeat default parameters in the
> docstring it it something we can get wrong, if we don't repeat them they
> can't be wrong.  What do we gain?

I wouldn't be surprised if some editors (Eclipse? Others) use or will use
this syntax for tooltips, to help users when editing.

	Bert


Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> Geez. Be smart, Philip. Obviously, that quoted content is dumb. I'm talking
> about format. Specifically not following the basic rule of "one line
> summary. One blank line. Details." Julian's change is missing the blank
> lines, which many tools parse.

That's what I don't understand.  Are we expecting people to use tools to
parse these docstrings?  If we have to repeat default parameters in the
docstring it it something we can get wrong, if we don't repeat them they
can't be wrong.  What do we gain?

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Greg Stein <gs...@gmail.com>.
On Mar 7, 2012 7:11 AM, "Philip Martin" <ph...@wandisco.com> wrote:
>
> Greg Stein <gs...@gmail.com> writes:
>
> > On Wed, Mar 7, 2012 at 05:27,  <ju...@apache.org> wrote:
> >> Author: julianfoad
> >> Date: Wed Mar  7 10:27:15 2012
> >> New Revision: 1297921
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1297921&view=rev
> >> Log:
> >> Improve doc strings of Sandbox.simple_*() test suite functions.
> >
> > If you're going to switch from single-line docstrings to multi-line,
> > then please follow PEP-8 guidelines for docstrings, which actually
> > just defers to PEP-257 on this matter:
> >   http://www.python.org/dev/peps/pep-0257/
>
> This example docstring:
>
> def complex(real=0.0, imag=0.0):
>    """Form a complex number.
>
>    Keyword arguments:
>    real -- the real part (default 0.0)
>    imag -- the imaginary part (default 0.0)
>
>    """
>    if imag == 0.0 and real == 0.0: return complex_zero
>    ...
>
> is just silly unless we are planning to make the docstrings available
> without the source code.  Duplication like that is pointless when
> reading the code.  What do we gain by writing docstrings in that form?

Geez. Be smart, Philip. Obviously, that quoted content is dumb. I'm talking
about format. Specifically not following the basic rule of "one line
summary. One blank line. Details." Julian's change is missing the blank
lines, which many tools parse.

Doxygen has formats. Python has PEP-257.

-g

Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> On Wed, Mar 7, 2012 at 05:27,  <ju...@apache.org> wrote:
>> Author: julianfoad
>> Date: Wed Mar  7 10:27:15 2012
>> New Revision: 1297921
>>
>> URL: http://svn.apache.org/viewvc?rev=1297921&view=rev
>> Log:
>> Improve doc strings of Sandbox.simple_*() test suite functions.
>
> If you're going to switch from single-line docstrings to multi-line,
> then please follow PEP-8 guidelines for docstrings, which actually
> just defers to PEP-257 on this matter:
>   http://www.python.org/dev/peps/pep-0257/

This example docstring:

def complex(real=0.0, imag=0.0):
    """Form a complex number.

    Keyword arguments:
    real -- the real part (default 0.0)
    imag -- the imaginary part (default 0.0)

    """
    if imag == 0.0 and real == 0.0: return complex_zero
    ...

is just silly unless we are planning to make the docstrings available
without the source code.  Duplication like that is pointless when
reading the code.  What do we gain by writing docstrings in that form?

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: svn commit: r1297921 - /subversion/trunk/subversion/tests/cmdline/svntest/sandbox.py

Posted by Greg Stein <gs...@gmail.com>.
On Wed, Mar 7, 2012 at 05:27,  <ju...@apache.org> wrote:
> Author: julianfoad
> Date: Wed Mar  7 10:27:15 2012
> New Revision: 1297921
>
> URL: http://svn.apache.org/viewvc?rev=1297921&view=rev
> Log:
> Improve doc strings of Sandbox.simple_*() test suite functions.

If you're going to switch from single-line docstrings to multi-line,
then please follow PEP-8 guidelines for docstrings, which actually
just defers to PEP-257 on this matter:
  http://www.python.org/dev/peps/pep-0257/

>...

Thx,
-g