Reg. packaging pam-kwallet for Debian

[Maximiliano Curia]

¡Hola Rahul!

El 2014-10-21 a las 01:26 +0530, Rahul Amaram escribió:
> Apologies for the delay. Had been caught up with some work.
> Kindly review and upload to Debian.

The review process involves checking and fixing the packaging, and checking
upstream code for possible errors/incompatibilities with the way things are
done in the distribution. It takes time from both of us.

My ultra motive to offer you to review the package is to have more members
engaged in the team, not to push things that are not up to the quality expected
in Debian.

> Version: 0.0~git20140429-1

There are a couple of fixes in the upstream git, last commit is 2014-05-08,
you might want to include those.

> Maintainer: Rahul Amaram <amaramrahul at users.sourceforge.net>

To be under the kde team umbrella the package should be something like:
Maintainer: Debian/Kubuntu Qt/KDE Maintainers <debian-qt-kde at lists.debian.org>
or:
Maintainer: Debian KDE Extras Team <pkg-kde-extras at lists.alioth.debian.org>
or:
Maintainer: Debian Krap Maintainers <debian-qt-kde at lists.debian.org>

The field: XSBC-Original-Maintainer is not considered valid in Debian
packages.

Add add yourself to the Uploaders list.

In the debian/copyright file:
Source: <url://example.com>
Please update the template to point to the upstream git repository.

Also in the debian/copyright file, the debian/* path is licensed under a more
restrictive license than the upstream code (GPL, and LGPL respectively), this
kind of licensing could block patches in the debian package from ever be
applied upstream and should be avoided. I pinged Rohan about this.

In Debian the pam modules are named libpam-$module, please rename the binary
package.

The description provides almost no information, please extend it. Consider
using the kwalletmanager description, and adding a paragraph about the pam
module (ala libpam-gnome-keyring).

It's a good idea to set the build dependencies versions to (at least) the ones
listed in the CMakeLists.txt, in this case cmake (>= 2.8.8) and
libgcrypt11-dev (>= 1.5.0).

In the code I don't see any obvious errors, but I'm not an expert in pam
modules, some comments though:
In kwallet_hash, after the call to error = gcry_kdf_derive(..) it's not
checking in error returned something.

In prompt_for_password, the memset in the lines:
    struct pam_response *response = NULL;
    memset (&response, 0, sizeof(response));
is redundant.

Also, the normal review process is done via mentors.debian.net, where you
could upload the package and send a RFS, I prefer using a git repository where
I can see the changes made, and afterwards integrate the changes in a repository
for the package, either one is fine, or even an uri where I can fetch the
package source (I don't care about the binary file).

In any case, I would prefer not to have the packages as attachments, specially
in bugs and the team mailing lists, so, unless you can't publish the files
somewhere else, please avoid sending them like so. And if you really have to
send the files as attachments, please send them via direct mail, without
copies.

Thanks,
-- 
“There are two ways of constructing a software design.  One way is to make it
so simple that there are obviously no deficiencies. And the other way is to
make it so complicated that there are no obvious deficiencies."
-- C.A.R. Hoare
Saludos /\/\ /\ >< `/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-kde-talk/attachments/20141021/0c93cec8/attachment.sig>