Bug#849754: Review
I reviewed this packaging and came up with some issues:
- The description does nothing to explain what makes this solution unique.
+ Why is this special?
+ How does it even work?
+ This should be easily gleaned from the package description.
- There is a lintian override for a legitimate mistake (unindented list).
- d/patches is scary...
+ Do not create lintian overrides using d/patches.
+ Keep patches to essential changes (not syntax and language changes).
+ Do not perform such a massive application rewrite using d/patches.
+ That change should be sent upstream... not held local.
- Absolutely DO NOT generate lintian-override files from d/patches.
- d/control should wrap long lines.
- d/rules could use some more whitespace.
- The py2 dependency is bad (very bad, especially at this point).
+ There are only two years left until py2 EoL.
- d/links suggests binaries are not being installed into standard locations.
- d/NEWS is not needed here and will be needlessly noisy for users.
- d/install should use wildcards to grab docs, instead of a file list
- d/install does not need the leading "./"
- d/compat should probably get a bump to 11
+ in d/control as well
- This is a python package, but raw source is being installed to /usr/lib.
+ The python scripts should be compiled.
+ There should be no "src/*" inclusion in d/install.
- Using d/install is not the correct way to include init script.
- The debhelper scripts have already been commented on.
- The interaction with the init system was also already commented on.
- This packaging doesn't follow any sort of standard.
+ There is no way to simply grab the source and build a package.
+ I would suggest creating a debian packaging repo (on alioth or salsa).
+ Using gbp will make it substantially easier for others to review.
+ Following a standard also means others can easily hop in and help.
- Tests are included, but nothing is done with them.
+ If you built them, use them!
Not Debian-centric:
- Why is there no Makefile?
+ What Install.txt provides should be available via make.
+ It's also where you would stick bits of code to generate documentation.
- The "release" version v0.0.0 suggests incomplete work.
+ There has been no release since v0.0.0 (in 2016), despite code changes.
- Why are man pages under debian/?
+ Make them a part of the software, not the package.
+ Generate the file man page during build (using make).
Personally, I don't understand what this backup solution offers over others.
Even after discussing this on IRC, I still really don't have any idea what it
offers over other solutions. I feel like a lot of buzz words are being
arbitrarily assigned to the description. This makes it hard to follow what is
fact and what is opinion.
Perhaps:
Guerilla Backup is a backup solution for small and unstable networks. It can
use your choice of transports (ssh, telnet, xmpp, etc.) to push backups to a
distributed cluster. Guerilla Backup ensures security by pre-encrypting data
before upload and only this key can be used to decrypt the data.
There's no real distinction between opinions, technical info, or general user
info. It makes it pretty much impossible to know what someone is getting with
this software.
Feel free to reach out via IRC if you need help!
--
Michael Lustfield
Reply to: