Re: Request for review: python-apt inline documentation
Hi Julian,
Julian Andres Klode wrote:
> Please note that this documentation is targeted at
> advanced developers and not at normal users (it's
> API reference).
I am not an expert in APT, Python, or English, but I think the cause
is a good one, so here are some thoughts on the first 10% or so as a
naïve reader.
> NAME
> apt_pkg - Classes and functions wrapping the apt-pkg library.
>
> FILE
> /home/jak/Desktop/python-apt/debian-sid/apt_pkg.so
>
> DESCRIPTION
> The apt_pkg module provides several classes and functions for accessing
> the functionality provided by the apt-pkg library.
Is the apt-pkg library at /usr/lib/libapt-pkg-<something>, as provided
by the apt package? Where can I find out more about it?
> It allows parsing of
> index files, configuration files, installation/removal of packages and
> much more
How were these items chosen? If they are meant to be the typical uses,
we can say that directly:
Typical uses might include reading APT index files and
configuration files and installing or removing packages.
> CLASSES
> __builtin__.object
> Acquire
[...]
> Version
>
> class Acquire(__builtin__.object)
> | Acquire(progress: apt_pkg.AcquireProgress) -> Acquire()
> |
> | Create a new Acquire object.
Yes, but what is it for?
An Acquire object is used to retrieve files from a remote
repository.
> | The parameter 'progress' can be used to
> | specify an apt_pkg.AcquireProgress() object, which will display the
> | progress of the fetching.
“Progress” and “fetching” are generally considered amorphous
substances that don’t deserve an article. Maybe one of the
two phrases below would be clearer:
which will display progress information during the fetch.
which will be used to report progress to the caller.
> | run(...)
> | run() -> int
> |
> | Run the fetcher and return one of RESULT_CANCELLED,
> | RESULT_CONTINUE, RESULT_FAILED.
Start fetching. Returns RESULT_CANCELLED, RESULT_CONTINUE, or
RESULT_FAILED, depending on how well that went.
What do the result codes mean? Does RESULT_CONTINUE represent
success or does it mean the operation didn’t finish yet? What
happens if there is nothing to fetch?
> | shutdown(...)
> | shutdown()
> |
> | Shut the fetcher down, removing all items from it.
What does it mean to remove all items from a fetcher? If I
restart the same fetch later, will it be able to use the partial
result?
> | fetch_needed
> | The total amount of data to be fetched.
Units? (Or does the type indicate that already?)
> | items
> | A list of all items as apt_pkg.AcquireItem objects.
Does this include items already fetched?
[...]
> | workers
> | A list of all active workers as apt_pkg.AcquireWorker objects.
Do these correspond to threads? How would a caller interact with
them?
[...]
> class AcquireFile(AcquireItem)
> | AcquireFile(owner, uri[, md5, size, descr, short_descr, destdir,destfile])
> |
> | Represent a file to be fetched.
For Acquire, you described what the constructor does; here, you
are describing what the object does. It would be nice to follow
whatever is standard practice in wording these things for Python
classes; unfortunately, I don’t know what the usual convention
is.
[...]
> | The destination can be specified by either 'destdir'
> | for specifying the destination directory or 'destfile' for
> | specifying the path to the destination file.
The destination can be specified as a directory using the 'destdir'
parameter or as a path to a file using 'destfile'. At most one
of these parameters should be not equal to None. If no
destination is specified, then...
> |
> | The parameters 'short_descr' and 'descr' can be used to specify
> | a short description and a longer description for the item. This
> | information is used by progress classes to refer to the item.
How long should these descriptions be? (e.g., a few words, one line,
one paragraph)
[...]
> | All parameters can be given by name (i.e. as keyword arguments).
Nice.
[...]
> | id
> | The ID of the item. An integer which can be set by progress classes.
>From the point of view of AcquireFile(), is this just a place
for callers to store some data, or is it used as a parameter to
some callbacks?
> | is_trusted
> | Whether the item is trusted or not.
Trusted by whom? I guess this indicates whether it was signed using a
signature in the APT keyring. Or "trusted according to the APT
configuration".
> | local
> | Whether we are fetching a local item (copy:/) or not.
> |
> | mode
> | A string indicating the current mode e.g. 'Fetching'.
punctuation: s/ e.g. /; e.g.,/ (though I don’t mind if you leave
out the comma).
> | partialsize
> | The amount of data which has already been fetched.
units?
> | status
> | The status of the item.
How does it compare to mode? (At first, I guessed this had something
to do with the status of a package.)
> | STAT_AUTH_ERROR = 4
There was an error in the credentials used to fetch the file.
Why is this separate from STAT_ERROR?
> | STAT_DONE = 2
> |
> | STAT_ERROR = 3
> |
> | STAT_FETCHING = 1
> |
> | STAT_IDLE = 0
When does the state change between STAT_IDLE, STAT_FETCHING, and
STAT_DONE?
> class AcquireItem(__builtin__.object)
> | Representation of an Acquire item.
Now another convention (last two were verbs, this one’s a noun). Maybe:
Abstract class representing a item to be fetched by an Acquire
object.
> | It is not possible to create
> | instances of this class, but it is possible to create instances
> | of the AcquireFile subclass.
Instances of the subclass are instances of this, too, right? Then
It is not possible to construct instances of this class directly.
Prospective users should construct instances of a subclass such as
AcquireFile instead.
Why does this class exist separately from AcquireItem? Is it possible
to make my own subclass with different behavior?
> class AcquireItemDesc(__builtin__.object)
> | This class provides the description of the
> | item and the uri the item is fetched from. It is used in progress
> | classes to get the descriptions to display.
language tweak: s/in/by/
[...]
> | uri
> | The URI from which to download this item.
Maybe:
The URI from which this item would be downloaded.
> class AcquireWorker(__builtin__.object)
> | Acquire workers represent exactly one subprocess used for fetching
> | files.
Another convention. :)
Presumably each AcquireWorker object represents one process, rather
than all of them taken together representing one.
> | This subprocess is created from the methods stored in
> | Dir::Bin::Methods.
language tweak: s/from the methods stored in/using methods from/
> | Data descriptors defined here:
> |
> | current_item
> | The item currently fetched, as an apt_pkg.AcquireItemDesc object.
The fetch of the item is in progress, so:
The item currently being fetched, ...
> | current_size
> | The amount of data fetched for the item.
The amount of data fetched so far for the current item.
> | resumepoint
> | The amount of data which was already fetched prior to resuming the
> | download.
The amount of data which was already fetched when the download was
last resumed.
> | status
> | The status of the worker, as a string.
This string would be a few words long, I assume.
> | total_size
> | The total size of the item.
Okay, this seems like a good place to stop for now.
Thanks for the pleasant read.
Jonathan
Reply to: