Hi Geoff,
Gianfranco already beat me to the review; nevertheless, here are some
additional notes I had prepared, based on the package I saw on Sunday
(the package is no longer visible on mentors.d.n).
On 2015-09-14 12:32, Gianfranco Costamagna wrote:
> lets review:
>
> 1) you dropped 0.10.0-2 entry from changelog
> 2) entry 0.10.0-1 is targeted wrongly
> 3) bump compat level not mentioned in changelog
> 4) according to setup.py
> install_requires=['python-musicpd>=0.4.1', 'requests>= 2.0.2'],
>
> so I guess there is no need to specify them to runtime dependencies
> (please check)
>
> 5) please mention copyright updates
> 6) mpd-sima.default
> "+## NOTA BENE:"
> this seems to be italian, please use english
>
> 7)
> +## only works with SysV init
> +## With systemd init: "touch /etc/mpd-sima_not_to_be_run" to prevent mpd-sima from being started
>
>
> well, can't you use some systemd facilities to do the same?
>
> http://www.freedesktop.org/software/systemd/man/systemd.unit.html
> http://www.freedesktop.org/software/systemd/man/systemd.service.html
>
> (or make systemv script behave in the same way)
d/control:
- The Vcs-Browser URL refers to the gitweb viewer, whereas the current
viewer seems to be cgit (the gitweb URL just redirects there)
d/changelog:
- typo: convertion -> conversion
- When bumping S-V, while not necessary, it is good practice to
indicate what changes were made in the process, or "no changes
needed" if that was the case
- It is helpful to be more explicit about some changes. You mention,
for example, that the package has been converted to Python 3. The
fact that the Python 2 package has been dropped is merely implied.
That is IMHO a significant change, and should be worth a hint
d/copyright:
- The Upstream-Source URL lists something completely different to the
the Homepage field in d/control. Could it be that one of them needs
to be updated?
d/NEWS:
- The upgrade path suggestion for the conffile in /etc isn't
really the prettiest solution, although I really don't know what
other path I could suggest that wouldn't seem like overkill. How
did upstream handle this change? Do they perhaps have a script or
tool that could assist in this process?
d/<somewhere>
- I encountered a lintian warning when building your package
Apologies for not providing more accurate references and/or possibly
outdated information. I had your package source in /tmp, which of course
didn't survive a reboot...
> (note, some of them might be not issues or just nitpicks, feel free to tell me so if
> you think they are good that way)
Same here.
Attachment:
signature.asc
Description: OpenPGP digital signature