Hi Raphael,
thanks for sharing your opinion with me.
On Wed, Jan 25, 2012 at 11:28:35AM +0100, Raphael Hertzog wrote:
> - the patch is much too big for a simple functionality like this one, you
> have to cut some code away, there are useless checks (code will end up
> failing if users submit something that's not expected, you don't have
> to hardcode checks for all possible mistakes that user might make),
> there are too many classes and intermediary objects, etc.
> Do your best to be "concise" and "readable".
I will not comment the "en detail" critique for now, since I figure we
have different design conceivabilities. We should discus about this first.
So lets go:
When I originally designed the library (and dpkg-report) I had the following
queries in
mind:
- Which actions happened in a given logfile?
- Which actions (...) in a given timerange?
- Which actions happened to a certain package?
- What is the final state (installed, half-configured) of a package at
the end of the logfile?
- What is the installed version of a package at the end of the logfile?
- What was the installed version of the papckage at the beginning of the
logfile?
- What time range does a logfile cover?
When analysing the format of the existing logfiles of systems where I
needed this, I figured that a logfile contains several entries
describing either the status of the dpkg run at a whole, the status of
an affected package or an action happening on a given package.
To answer the queries I figured that I need a lot of information about
different entities, like entries and packages (and conffiles) with
different attributes and different methods to work with. For example,
one who wants to analyse a logfile with different queries - which I can not
know in advance - will want to work with a line-oriented module like
Dpkg::Log::Status, which will return parameterized objects of each line,
while somebody who wants to do common queries (like those above), is
better off with something already doing the tracking work this
requires.
Ulimately I think Dpkg::Log and Dpkg::Log::Analyse are logically for different
tasks (low- and high-levell) and therefore need to be separate.
You seem to have another opinion, but I'm missing a rational.
Just reducing the number of modules does not seem to cut it.
> Again, I don't see the need for this module. It's doing basic queries
> on Dpkg::Log in a way that's not generic enough to be suitable for all use
> cases that applications that might have.
I stronly disagree on this. Yes, it is doing "basic" queries (in the
sense of queries most typical use cases will involve), yet those
queries are not simple (states need to be tracked) and so applications
shouldn't have to-reinvent them everytime.
> > + if ($entry[2] eq "update-alternatives:") {
> > + next;
>
> update-alternatives no longer writes to dpkg.log.
Maybe. But older logfiles exist and might get processed for whatever
reason.
> All the handling of attributes on *::Entry could be generalized
> and stuffed into the base class if it's really only a storage
> class. Using dedicated methods like $entry->type() doesn't bring anything
> compared to $entry->attribute("type").
Well, it at least brings a well-defined API, IMHO.
-Patrick
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
