[pkg-php-pear] php-analog review

Sat Feb 22 00:37:34 UTC 2014

Hi David,

Le 21/02/2014 22:27, David Prévot a écrit :
> 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.

You're right, it's a good habit and dh_clean ay al. should be my friends...

>>> [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).

Good idea as descritpion is almost the same as the first paragraph of
long description.

>>> [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

Gosh, I'd started with Apache 2 before searching and finding this link,
but didn't read it deaper enough.

>> 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.

See the point thank you.

>>> [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

All I've done came from php-opencloud, and I was very proud to have
functionnal tests at build time. I need to investigate more.

>>> [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.

After a (short) look, I feel DEP-12 is'nt quite stable, could we stay
for this release on this really painfull patch, providing I try to make
an idea on DEP-12 ?

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

Thank you.

>> 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.

He is the main author but not the sole, should I just add

Files: examples/SplClassLoader.php
Copyright: Jonathan H. Wage <jonwage at gmail.com>

without any License: stanza ?

>> 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.)

Reformulation of "restart from scratch" :

$ ssh anonscm.debian.org rm -fr /git/pkg-php/php-analog.git
$ gbp-create-remote-repo [...] # from a clean local git repo

What I've done following your previous mail, I won"t make it anymore but
I could'nt see how to unmerge upstream  from  master neither to clean
the mess on pristine-tar.

>>> 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

Great, still a lot of thing to learn about pbuilder.

Thank you very much for your remarks David,

Cheers,

-- 
François-Régis

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 880 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-php-pear/attachments/20140222/af2cbb1d/attachment.sig>