[pkg-php-pear] RFS php-easyrdf 1.0.0-1

[Marco Villegas]

Robin, David,

Thanks for taking the time to review the new version of the package.

tl;dr New changes on the git repository addressing suggestions added.

## On Robin comments

> - You may want to sign your git tags

Indeed, signed tags are better.
I am a bit confused because all Debian related tags are signed.
i.e. 
debian/0.9.1-1
debian/0.9.1-2
debian/0.9.1-3
debian/0.9.1-4
debian/0.9.1-5

For the two upstream/ namespaced tags, I guess gbp does not sign by
default, I changed configuration for future imports.

For the upstream tags, they are not always signed, but that is out of
our control.

For the curious, this can be added via --sign-tags, e.g. change your
gbp.conf to have:

    [DEFAULT]
    sign-tags=True

> - You may want to include and use upstream's test suite.

Please notice that as explained at the initial ITP bug[1], the test
suite could not be used because the required version of phpunit was too
old, it was requiring ~3.5. That was my initial motivation to add smoke
tests that are part of the package.

In other words, it was not possible to run phpunit test suite for
easyrdf on versions prior 1.0.0 on the package, since Debian had a
phpunit newer than upstream supported (quite strange fact to get into
BTW, related to upstream inactivity for some years).

This has changed on the newly packaged 1.0.0, where the required
version is ^7, so I have opened a related bug at the BTS[2].

> - The .sh files should be set to executable and "Test-command: sh
> path/to/file.sh" should be just "Tests: file.sh"

Indeed, that is possible.
If I am reading this correctly, this is mainly aesthetics, and maybe a
tiny Debian infrastructure-side performance optimization on package
building. Is that the case?

I am curious for the "should", is that part of policy somewhere that I
missed? If so, should we implement a lintian warning about it?

> debian/autoload.php.tpl
> - Looks like one of phpab's default templates. If so, it can be
> removed in favor of just letting phpab handle it.

Right, using the default template works OK, so I have removed the
template.

> debian/control
> - php-mbstring isn't necessary in Depends as it's picked up
> automatically from composer.json.

+1, removed.

> debian/docs
> - CHANGELOG.md should probably be installed by dh_installchangelogs in
> override_dh_installchangelogs instead.

Already there, see reply below.

## On David comments

> > - You may want to include and use upstream's test suite.
> Agreed. Upstream has a “test-php8-compatibility” branch on top of its 
> latest release that has compat with recent PHPUnit, so there should
> be little to fix in order to make it work during build and CI
> (skipping test related to ml/json-ld not yet in Debian for example).

I was thinking in php8 compatibilty and recent phpunit support as
different tasks, but they seem to be related.

I have just added a related bug reported[2] with more details about
phpunit support.

I am guessing this comment may be in preparation for php8 in
experimental that may get into bullseye. Is there something I may be
missing about php8 compatibility?

> > debian/tests/control
> > - The .sh files should be set to executable and "Test-command: sh
> > path/to/file.sh" should be just "Tests: file.sh"  
> 
> I believe those tests are trivial, and thus must be marked as 
> superficial as recently pointed by the release team. As is, the
> package is RC-buggy, so needs to be fixed.
> 
> https://release.debian.org/bullseye/freeze_policy.html

I did not know about the autopkgtest's superficial restriction[3].
And yes, those test are intended as minimal functional/smoke tests.
I have added superficial restriction to them.

> > debian/autoload.php.tpl
> > [...]
> > - CHANGELOG.md should probably be installed by dh_installchangelogs
> > in override_dh_installchangelogs instead.  
> 
> No override even needed: the upstream changelog gets picked up 
> automatically by dh_installchangelogs (as 
> /u/s/d/php-easyrdf/changelog.gz), but anyway, agreed on all your
> other remarks.

Yes, changelog is already part of the package since the initial
release[4].

> I did not upload the package (because of the superficial tests not 
> marked as such), nor did I review the upstream changes (will do once
> the package is fixed, eventually updated to a newer upstream version).

I will try to get into 1.1.0 updates for the package, but I would
suggest to go with the currently already packaged 1.0.0 for now.

Please find new commits at the salsa repository[5] addressing most of
the suggestions from both Robin and David.

1: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=932244#15
2: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=980389
3: https://salsa.debian.org/ci-team/autopkgtest/raw/master/doc/README.package-tests.rst
4: https://snapshot.debian.org/package/php-easyrdf/0.9.1-1/
5: https://salsa.debian.org/php-team/pear/php-easyrdf

Best,

-Marco