[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: solarpowerlog (improved...)



Hello,

On Sun, Jan 1, 2012 at 5:26 PM, Tobias Frost <tobi@coldtobi.de> wrote:
> However, please go forward with the review of the package, and to ease
> things I'd appreciate if a DD steps forward and indicated his/her will
> to sponsor this package, to define clear goals and clear actions whats
> needed to get this package into debian and avoiding wasting effort by
> focused work.

As a non-DD, here's a basic review:

* There's trailing whitespace in debian/changelog, debian/rules and
possibly other places as well (didn't check all files).

* Your debian/changelog might be too verbose. You want to describe the
changes since the last upload. Since it's a new package entering the
repositories, you might want to use a single entry like "Initial
release (Closes: #1234)".

* I'd tweak the short description a bit. How about "photovoltaic data
logger" instead? Take a look at the rsyslog short description for
inspiration.

* Get rid of "debian/dirs". If you're installing to /usr/bin, you
don't need to specify it [1].

* debian/rules has the sample header ("Sample debian/rules that uses
debhelper...") and some leftovers from sample comments. Get rid of
them.

* I only glanced over your rules file. Personally, I find short-form
debhelper much easier to maintain and review, I'd highly recommend it.

* There's a stray "SEE ALSO" section in your manual page. Take a look
at man-pages (7) for further advice, there's a lot that could be
improved in the manual page. Also, you probably want to submit it
upstream so it's maintained in sync with the program it describes and
can be easily used by people packaging the software for other distros.

* You might want to enable hardening flags [2].

I didn't build or test the package. I tested the watch file, but as
you described in the changelog, currently it doesn't work (not sure
what would be the right thing to do in this case).

Regards,

[1]: http://www.debian.org/doc/manuals/maint-guide/dother.en.html#dirs
[2]: http://wiki.debian.org/Hardening


Reply to: