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

Re: Request to review and upload unhide 20210124-1



Hello Daichi,

Here's my review, the first 3 items are more important, the other ones
are suggestions on further improvements you can do if you have the
time:

1) d/copyright:
I can see upstream has updated the copyright years and assignments on
a few files by looking at the following commit:
https://salsa.debian.org/dfukui/unhide/-/commit/ce1d2cac78695f8373fe1407e666869662ffd38e

So we need to update the definitions under d/copyright. You could
simplify that file by stating the following:
Files: *
Copyright: 2005-2021 Yago Jesus <yjesus@security-projects.com>
                  2010-2021 Patrick Gouin
License: GPL-3+

And removing the entry for the "sanity" files.
But of course you can always be more precise and declare each file as it is.

2) branches upstream and pristine-tar:
I believe you forgot to commit those branches, could you do that?

3) d/control:
We're missing a dependency on procps, and probably on other packages
too (as mentioned in "Require" section of the README).
Due to the lack of procps, I can see autopkgtests are failing.
Let me know if by any chance autopkgtest is passing on your
environment so I can try to figure out why.
And if you're not running that, you can either take advantage of
salsa-ci or make sure your build tools run it (sbuild supports
autopkgtest), let me know if you'd like me to expand on that.

Extra things you could do to improve packaging even more:

4) Python client:
I see that unhide now offers a python client, with a GUI: unhideGui.py
My suggestion is to consider shipping that GUI as well, but it will
require some testing to make sure it's working fine.

5) DH bump to 13:
debhelper-compat could be bumper to 13 in d/control

6) d/control: Standards-Version:
You could go through the upgrade checklist of the Debian policy and
check if it could be bumped to 4.6.0:
https://www.debian.org/doc/debian-policy/upgrading-checklist.html

7) d/unhide.postinst:
This file can be removed since unhide 20110113-3 was superseded 10 years ago.

8) d/unhide.manpages:
I'm not sure this file is needed as debhelper should be smart enough
to catch it, but I didn't double check.

Thank you for working on this, I'm sure this will benefit a lot of users :)


--
Samuel Henrique <samueloph>


Reply to: