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

Re: PmWiki



On 08/26/2011 03:41 AM, Strobl, Robert wrote:
> Hi Thomas,
> 
> your referred package is not the one I am maintaining.
> Mine can be found at http://share.gloriabyte.de/pmwiki/ and is
> lintian clean.
> 
> Best regards,
> Robert

Hi Robert,

here's my review. The package is generally in a very good shape. I would
have upload it if there wasn't an issue with your debian/copyright.
However, I think there is room for improvement.

1/ Your debian/copyright is wrong. You can't just write:

License: GPL-3
 see "/usr/share/common-licenses/GPL-3".

you need to put a copyright notice for the GPL-3 as well. Also, it's ok
to use the license you like for the packaging, but generally, it's best
to use a license at least not more restrictive than the upstream one,
and even better if you use the same license as upstream. That's just a
suggestion though, but I would simply use GPL-2 like the upstream
license. What do you think?

2/ In your postinst, you are doing:

    ln -s /var/lib/pmwiki /usr/share/pmwiki/wiki.d
    ln -s /etc/pmwiki /usr/share/pmwiki/local

Why don't you just use dh_link and ship the symlinks in your package
directly? That would also avoid the need of doing:

rm -rf /usr/share/pmwiki/

in your postrm.

3/ rm -rf /etc/pmwiki/ in your postrm is a bad idea. What if a user
wants to writes notes in /etc/pmwiki/my-nice-notes? You'd be deleting it
on the purge target, which the user didn't want.

I would also suggest that you write a very documented
/etc/pmwiki/config.php with in it things like:

<?php if (!defined('PmWiki')) exit();
# Define here the title of your wiki
#$WikiTitle = "Example title";

# path to your logo
#$PageLogoUrl = "/path/to/logo.gif";

# URL of your wiki
#$ScriptUrl = "http://pmwiki.example.com";;

# Password for the admin
#$DefaultPasswords['admin'] = crypt('XXXX');

# Set to 1 to enable uploads
##$EnableUpload = 1;

# Define a password for the uploads
#$DefaultPasswords['upload'] = crypt('XXXX');

# Name of your skin, corresponding to the folder inside pub/skins
#$Skin = 'gplhost';

# Set the timezone format
#putenv("TZ=UTC");
#$TimeFmt = '%B %d, %Y, at %I:%M %p EST';

# Use "Clean URLs".
#$EnablePathInfo = 1;

I know the above isn't mandatory, but it would help A LOT to have it.
You could also copy the sample-config.php into /etc/pmwiki/config.php if
you see that the file doesn't exist (and delete it in case of a purge of
course). Best would be to use ucf, but that might be a bit
over-engineered ... I'll leave that choice to you.

Also, have you considered a strategy to package the myriads of modules
and extensions to pmwiki? I do use myself:
- The captcha thing
- The internationalization modules
- The URL approve script

4/ While checking for your watch file, I saw there's a new version
upstream. I understand that the issue is because of me taking too much
time to review your package, but it'd be nice that on your next upload,
you'd be upgrading.

uscan --verbose --report
Newest version on remote site is 2.2.30, local version is 2.2.29

5/ Is that one needed?

I: pmwiki: package-contains-empty-directory usr/share/pmwiki/pub/css/

6/ Instead of your debian/patches, I would suggest to override the
dh_install target and do something like:

find cookbook local scripts wikilib.d pmwiki.php -exec \
	install -D -m 0662 {} debian/pmwiki/usr/share/pmwiki/{} \;

and ship your index.php inside the debian folder instead of a patch. Of
course, you'll have to also do that for the docs folder. That way, you'd
get totally rid of the debian/patches folder.

I know I might have tell you to create that Makefile (if that was the
case, sorry for the bad advice), but on the 2nd though, I think it's
better as above, directly in debian/rules.

7/ Last thing, it'd be cool to use a VCS for your packaging. If you use
collab-maint and Git on Alioth, I might well also help for the
packaging, since I think that pmwiki is a cool software and I use it as
well.

Cheers, and thanks a lot for your packaging effort,

Thomas goirand (zigo)


Reply to: