On Sun, 2009-04-19 at 18:22 +0200, Jan Hauke Rahm wrote:
> Okay, seems like I needed to inversely (is that a word?) sort the hash
> keys. A new patch is attached. If you, Paul, find again some time
> testing would be much apppreciated. I'll leave this topic now to give
> the actual maintainers a chance to review what I've done to the
> formerly nice readable code. :)
The patch itself seems fine in general, and Paul's obviously given it at
least some testing. On that basis, most of my comments may be a little
picky again; sorry about that. :-) (Feel free to ignore or disagree
with any of them, as should anyone else reading them).
> + --pc-vote Sort by_vote instead of by_inst (see debtags(1))
popularity-contest(8) ? :-)
> + open POPCON, "/var/log/popularity-contest"
> + or die "Unable to access popcon data: $!";
[...]
> + or die "Not able to receive remote popcon data!";
die "$progname: foo", please.
> + if ($pkg_store{$index}) {
> + $pkg_store{$index} .= $bug_string;
> + } else {
> + $pkg_store{$index} = $bug_string;
> + }
fwiw, this version:
+ $pkg_store{$index} .= $bug_string;
works for both cases. This is one case where Perl doesn't care that
$pkg_store{$index} hasn't been initialised before the concatenation.
As a general note, there are a number of places in the patch where
either the indentation of existing code has been changed, mainly by
replacing tabs with spaces (please don't do that ;-), or new code is
intended to the wrong level. The rule of thumb is that each indent
level consists of four spaces, with a run of eight spaces becoming a
tab.
> +Popularity-contest collects data about installation and usage of
> Debian
> +packages. You can additionaly sort the bugs by the popcon rank of the
> related
"additionally"
> +.BR \-\-popcon
> +Sort bugs by popcon rank of the package the bug belongs to.
"by the"
> +.TP
> +.BR \-\-pc\-vote
> +Packages are sorted by the number of people who installed this
> package. With
> +this option you can change it to the number of people who use this
> package
> +regularly. This option has no effect in combination with \-\-pc
> \-local.
Maybe "By default, packages are sorted according to the number of people
who have the package installed. This option enables sorting by the
number of people regularly using the package instead." ?
> +.TP
> +.BR \-\-pc\-local
> +Instead of requesting remote data the information of the last popcon
> run is
> +used (/var/log/popularity-contest).
"information from"
> +Read out /var/log/popularity-contest and sort bugs by your personal
> popcon
Remove "out".
> +ranking (which is basically the atime of your packages binaries).
"packages'", assuming it's being used in the plural (which it appears to
be).
popularity-contest(8) probably also wants adding to "see also".
Regards,
Adam
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
