[MERGE] improvements to the docstrings of bzrlib.smart and contents.
Aaron Bentley
aaron.bentley at utoronto.ca
Sat Oct 27 21:10:11 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Michael Hudson wrote:
bb:resubmit
> +At the bottom level there the protocol consists of bytes,
Two things: "there the protocol" looks grammatically wrong. And I don't
think the original intent was to call the raw bytes a protocol.
which are
> +transmitted over a socket, a pair of pipes, or an HTTP request/response. We
> +call this layer the *medium*.
This makes it sound like the bytes are the medium, not the method used
to transmit the bytes. So this actually reduces clarity IMHO.
> These different options have different
Let's call them "media" or "mediums", not "options".
> +behaviors:
> +
> + - You can pass multiple requests over a socket and you get a read error when
> + the other side has shutdown.
I'm not sure about "shutdown", since it's not a normal word. We could
say "has shut down". Or we could say "done a shutdown" if we want to
telegraph the fact that we're referring to the socket function.
> + - When the other end of a read pipe has gone away and you try to read from
> + it, you get a zero bytes which indicates 'end-of-file'.
In Python terms, it might make more sense to say "empty string" than
"zero bytes".
> +
> + - For HTTP, each request is independent and so there no concpet of
> + 'end-of-stream'.
"concept" misspelled.
> So we need a wrapper around pipes and sockets to seperate out requests from
"separate" misspelled.
> -On top of the medium is the *protocol*. This is the layer that deserialises
> -bytes into the structured data that requests and responses consist of.
> +On top of the medium is the *protocol*. This is the layer that deserializes
> +bytes into the structured data that make up requests and responses.
As a Canadian, I get to pick and choose between US and UK spellings, and
I prefer "serialize".
> -PROTOCOL (serialization, deserialization) accepts structured data for one
> + PROTOCOL (serialization, deserialization) accepts structured data for one
At least be consistent about whether it's "deserialization" or
"deserialisation".
> @@ -83,7 +83,8 @@
> """Initialize a bzrdir at path.
>
> The default format of the server is used.
> - :return: SmartServerResponse(('ok', ))
> +
> + :return: ``SmartServerResponse(('ok', ))``
We don't normally quote return values when they refer to objects. I
would really hate to start putting double-backticks around all our
objects. It's not returning the string "SmartServerResponse(('ok', ))"
or anything.
> === modified file 'bzrlib/smart/medium.py'
> --- bzrlib/smart/medium.py 2007-10-05 14:52:02 +0000
> +++ bzrlib/smart/medium.py 2007-10-25 12:03:45 +0000
> @@ -21,7 +21,7 @@
>
> Media carry the bytes of the requests somehow (e.g. via TCP, wrapped in HTTP, or
> over SSH), and pass them to and from the protocol logic. See the overview in
> -bzrlib/transport/smart/__init__.py.
> +`bzrlib.smart`.
Single backticks are definitely wrong, unless you're trying to make it
italic.
> - The stream may be a pipe connected to sshd, or a tcp socket, or an
> - in-process fifo for testing.
> + The stream may be a pipe connected to sshd, or a tcp socket, or an in-process
> + fifo for testing.
This is just changing the line-wrapping, right?
> - The server passes requests through to an underlying backing transport,
> - which will typically be a LocalTransport looking at the server's filesystem.
> + The server passes requests through to an underlying backing transport, which
> + will typically be a `bzrlib.transport.local.LocalTransport` looking at the server's
> + filesystem.
Augh, why all these backticks?
> - :returns: a SmartServerRequestProtocol.
> + :returns: a `SmartServerRequestProtocol`.
Why?
> - """Construct a SmartClientMediumRequest for the medium medium."""
> + """Construct a `SmartClientMediumRequest` for the medium ``medium``."""
I really do object to these backticks. In the context of API
documentation, you shouldn't have to write anything special when
referring to parts of the API.
> class SmartClientMedium(object):
> - """Smart client is a medium for sending smart protocol requests over."""
> + """A SmartClientMedium is a medium over which smart protocol requests can be sent."""
Dangling participles are frequently less awkward than the alternative
"grammatical" construction.
> The body is encoded with one line per readv offset pair. The numbers in
> - each pair are separated by a comma, and no trailing \n is emitted.
> + each pair are separated by a comma, and no trailing ``\\n`` is emitted.
> """
Molesting the \n like this reduces clarity. One might easily think that
the string is a backslash, followed by a line feed.
> def cancel_read_body(self):
> - """After expecting a body, a response code may indicate one otherwise.
> + """After expecting a body, a response code may indicate otherwise.
Neither the original nor the new phrasing is very clear to me.
> def execute(self, *args):
> """Public entry point to execute this request.
>
> - It will return a SmartServerResponse if the command does not expect a
> + It will return a `SmartServerResponse` if the command does not expect a
> body.
>
> - :param *args: the arguments of the request.
> + :param \*args: the arguments of the request.
I think I'd rather omit the asterisk than introduce a backslash. The
asterisk isn't really part of the variable name.
> @@ -173,8 +173,8 @@
> def _run_handler_code(self, callable, args, kwargs):
> """Run some handler specific code 'callable'.
>
> - If a result is returned, it is considered to be the commands response,
> - and finished_reading is set true, and its assigned to self.response.
> + If a result is returned, it is considered to be the commands response;
Should be "command's response", I think.
> + finished_reading is set true and its assigned to self.response.
Should be "it's assigned".
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHI5si0F+nu1YWqI0RAmopAJ9J/Jm6JjJGYij++3x7TTV6O4LfUgCcDjyT
VolP7ydXrSPrA0Erd8cBsno=
=hbzr
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list