Am Freitag, den 17.02.2012, 12:48 +0100 schrieb Benoît Knecht:
Sorry for the late reply, spare time was quite spare the last days...
> Hi Tobias,
>
> As promised, here's my review of your package:
>
> - In debian/solarpowerlog.default, you define /etc/solarpowerlog as
> the default RUNDIR; what files is your program expecting to find
> there, HTML templates? If so, installing some default templates in
> /usr/share/solarpowerlog and pointing your program to that directory
> seems like a better option.
The templates were in an earlier release at /etc/solarpowerlog, but I
moved them to /usr/share/docs/solarpowerlog/examples after I got
received some critism about the usage of the similie timeline widgets,
which are (if they are) only very hard to package.
So I won't install any default templates except as the said examples,
but I will remove the reference to the rundir and hint in the installed
example configuration file to use absolute paths.
> You also note there that start-stop-daemon is taking care of running
> solarpowerlog in the background, but according to
> start-stop-daemon(8), "this is a last resort, and is only meant for
> programs that either make no sense forking on their own, or where
> it's not feasible to add the code for them to do this themselves";
> since your program can put itself in the background, you should
> rather use that possibility. This way, it will also make sense
> checking the exit status of start-stop-daemon in do_start in the
> init script, because if you use its '-b' option, it can't determine
> if the process failed to execute.
Yes, the daemon support of solarpowerlog is a mixture of barely tested,
incomplete, limited, you name it.. At least up to version 0.22, as I
worked on this with the last commit to my development branch. In other
words, 0.23, which I plan to release soon, will get rid of the hacks you
mentined above.
> And it'd be best to wrap lines at 80 columns.
>
> - Running uscan tells me:
>
> Processing watchfile line for package solarpowerlog...
> Newest version on remote site is 0.21a, local version is 0.22
> solarpowerlog: remote site does not even have current version
I had an typo ("_" instead of "-") when I uploaded the latest tarball to
sf.net. Should work now.
> - I think your short description should read "photovoltaic data
> logger", as in "solarpowerlog is a ..."
>
> And in the long description, you wrote "photo-voltaic"; I think you
> should stick to the former spelling.
>
> The second paragraph of the long description is missing punctuation.
Fixed in my debian branch.
> - In the solarpowerlog(1) man page, the SEE ALSO section appears to be
> a subsection of OPTIONS.
>
> Please consider removing the AUTHOR section (see man-pages(7) for an
> explanation why).
>
> In DESCRIPTION, I would start with "solarpowerlog tracks and
> logs..."; it's not just its purpose, it's what it actually does,
> right? Same for the second sentence.
>
> In OPTIONS, you start with "These programs"; why the plural?
The easy things are already fixed, the complete rewording will take some
time ;-)
Especially as I added several command line options (thanks to the
improved daemon support) I will have to rework those manpage. Its a
pitty that I like coding more than wording, but it is defintly on the
list.
> Cheers,
>
> --
> Benoît Knecht
>
>
>
Many thanks for the feedback,
Bye,
coldtobi
--
To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive:
http://lists.debian.org/1330383271.13931.29.ca...@ithilien.loewenhoehle.ip
