Bug#701870: RFS: aspsms-t/1.3.1-1
On Thu, Feb 28, 2013 at 5:56 PM, Marco Balmer wrote:
> A long time ago, that I've sent a RFS request to debian-mentors [1].
> Accidentally the package was removed by mentors [2] and I didn't
> notice that. So I've uploaded it again. ITP bug for this package is
> available at [3].
Looks like it was removed on purpose not by accident.
> More information about hello can be obtained from http://www.example.com.
I guess we should remove this from the RFS template since no-one seems
to read it properly.
I don't intend to sponsor this package, but here is a review:
debian/rules can be replaced with
/usr/share/doc/debhelper/examples/rules.tiny, plus add --parallel to
the arguments of dh.
debian/docs looks bogus, there is no docs dir in the unpacked package.
The License field in debian/copyright should be GPL-2+ not GPL.
I would suggest running wrap-and-sort -sa to wrap debian/control and
other files.
lib/ASPSMS/GetNetworksFees.pm uses system() in a potentially unsafe
way since it passes variables through /bin/sh. Please change the code
to use multiple arguments to system() and it will then not use /bin/sh
and be more safe. The code seems to use a lot of backticks too, I
guess they aren't safe either. There is also the issue of arguments
being confused with options, add "--" to prevent that.
Don't need to use system('wget') in Perl code, please use LWP instead:
http://search.cpan.org/dist/libwww-perl/
Don't need to use system('mv') in Perl code, there is the rename function:
http://perldoc.perl.org/functions/rename.html
When I install the package I get this message, please silence it:
id: aspsms: no such user
Your package will leave files behind after it is purged, please fix
that. You also don't need to run id on every upgrade (just on initial
install). If the sysadmin deletes the files in /etc then they will get
recreated on upgrade, you're supposed to preserve even deletions. I
would suggest just installing it as a conffile per usual.
It doesn't really make sense to put the manual pages in the -perl package.
Please split the upstream README into README and README.install since
the build/install info will not be useful to people installing the
package.
I think the documentation should use /srv/ rather than /home/ in many places.
Since you are upstream, please read our upstream guide:
http://wiki.debian.org/UpstreamGuide
Automatic checks:
https://wiki.debian.org/HowToPackageForDebian#Check_points_for_any_package
lintian:
I: libaspsms-perl: spelling-error-in-manpage
usr/share/man/man1/aspsms-t.notify.1p.gz Absolut Absolute
I: libaspsms-perl: spelling-error-in-manpage
usr/share/man/man3/ASPSMS::Jid.3pm.gz ressource resource
I: libaspsms-perl: spelling-error-in-manpage
usr/share/man/man3/ASPSMS::Message.3pm.gz fuction function
I: aspsms-t: init.d-script-does-not-implement-optional-option
etc/init.d/aspsms-t status
W: aspsms-t: executable-not-elf-or-script
usr/share/doc/aspsms-t/examples/aspsms-t.xml.dist
I: aspsms-t: unused-override binary-without-manpage usr/bin/aspsms-t
perl -wc:
./examples/aspsms.SendWAPPushSMS.pl syntax OK
Reference found where even-sized list expected at
/usr/share/perl5/ASPSMS/config.pm line 57.
Reference found where even-sized list expected at
/usr/share/perl5/ASPSMS/config.pm line 58.
Perl::Critic:
Many complaints, no idea how valid they are.
--
bye,
pabs
http://wiki.debian.org/PaulWise
Reply to: