[pkg-php-pear] php-analog review

[David Prévot]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi François-Régis,

Le 20/02/2014 19:22, François-Régis a écrit :
> Le 15/02/2014 20:17, "David Prévot" a écrit :

>> Please “echo .pc > ~/.gitignore” instead of editing .gitignore of every
>> package.
> 
> Done, although I found it usefull for me to specify some files from
> package to package.

You can add more than one line in ~/.gitignore. I also used to edit
.gitignore in package repository, but found it was a needless overhead
on “gbp import-orig” when the .gitignore file is edited upstream.

>> [control]
>> More ${phpcomposer:… are available, please use them (even if they’re empty
>> now).
> 
> Done as much as I can, description in composer.json is a bit long so I
> don't keep it, perhaps it would be better to patch it and send patch
> upstream ?

Maybe. An alternative could be to use the description in composer.json
as (part of) the long package description (if it makes sense).

>> [copyright]
>> Please document Upstream-Contact.
>> At least lib/ChromePhp.php license should be documented.

Please name the licence Apache-2.0 (or Apache-2) instead of Apache as
advised:
https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/#license-specification

> I though it was preferable to have ${package}.install (just from what
> I've seen in some packages) but I'ts obviously sharper without prefix.

You may find useful to be able to copy and paste the whole debian/
directory as a basis when you start a new package that looks like one
you have already done (that can be pretty useful for php-* packages ;).
Not having to rename files makes the task a bit more straightforward.

>> [tests]
>> DEP-8 is about testing the installed package, not the source one.
> 
> Could you point me somewhere to start ?

I don’t have much experience in DEP-8, but feel free to look at the
workaround implemented in php-opencloud (improvements welcome):

http://anonscm.debian.org/gitweb/?p=pkg-php/php-opencloud.git;a=blob;f=debian/tests/phpunit;hb=HEAD
http://anonscm.debian.org/gitweb/?p=pkg-php/php-opencloud.git;a=blob;f=debian/patches/DEP-8/Use-installed-class-for-DEP-8-tests.patch;hb=HEAD

>> [upstream/changelog]

> Done as a patch (would it be better to directly mangle the web page ?),

Updating such patch may be painful on upgrade, why not directly include
this file into the debian/upstream/ directory (assuming DEP-12 is going
to evolve as discussed in #736760 and debian-devel@), and then override
dh_installchangelogs in debian/rules.

>> I’ll review the upstream part once you’ve double checked d/copyright.

On my way.

> Done : examples/SplClassLoader.php is authored by Jonathan H. Wage
> <jonwage at gmail.com> (https://gist.github.com/jwage/221634) and others
> but has no copyright notice. What do we do with it ?

If the sole author didn’t bother to indicate copyright, we may assume he
is the copyright holder.

> And for the review just tell me if I can restart from scratch the alioth
> repo.

I fail to understand that last sentence, can you reformulate please?
(Just in case I actually got the question right: please don’t rebase the
master branch anymore, unless you really need to.)

>> php-psr-log uploaded in the mean time, thanks.
> 
> Thank's, I can now pbuild php-analog.

You can simply add a local repository (from a simple directory) without
the burden of waiting for NEW processing (that can be handy for building
new stuff that depends on new stuff that depends on new stuff…):

https://wiki.debian.org/PbuilderTricks#How_to_include_local_packages_in_the_build

Regards

David


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJTB8SnAAoJEAWMHPlE9r08ApEH/2cFK2hl6qa3/C5AmxEHZ/8/
YDNARMtHWAr+0hk/BkW8PidJ6drKn7Bk8fHF4kFuc6XdJW2a517M6mCuSN6oLkeW
mfnGoxJEsYrisSlFP0QUn86sLeG9M9vmFYemXhydAQ+WA0/JERVDsNVV7Q9+eyob
Go02JD+wayQ/ATgOoD0cxpP7gokdw0MfimHa6fjXtwA37/SAmbTbtfeiaDTlIGhI
GnbcBf3Q+3VPGu1nB++QK8aICIKb1qPxtcyuyVE69LaGOVnGXjHwEzF9xV3qKVAg
WrO+GbqLQAFZMqzAEclFI7noeSITbgdh80ws8YaV1lbd1TQYLYFNZQlk6DJrxgI=
=bGVw
-----END PGP SIGNATURE-----