siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for dist-6E-epel failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031564

siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for f23 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031559

siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for epel7 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031562

siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031558

siwinski's scratch build of php-symfony-polyfill-1.0.0-1.fc23.src.rpm for f22 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12031560

siwinski's scratch build of php-symfony-polyfill-1.0.0-2.fc23.src.rpm for rawhide failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12033450

Damn.. I Haven't notice this review.

Again symfony does the stupid thing to merge everything in a single big repo... and then distribute various components from it...

See: https://github.com/symfony/polyfill/pull/15

I think it will be simpler to provide everything, whatever the PHP version is.

- this will be better with SCL.
- this will be noop (bootstrap will be ignored)
- this will avoid to have to add test in all consumer library

Ex: jeremeamia/superclosure requires symfony/polyfill-php56

See https://github.com/remicollet/remirepo/commit/5ac8cde9e3b7323bb01eaba76312529495296fe8

remi's scratch build of php-symfony-polyfill-1.0.0-1.remi.src.rpm for rawhide completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12080601

I see 2 possible options:

1) Always provide ALL polyfills: https://github.com/siwinski/rpms/blob/e64463828a7c70097d7f7b8463983cc570b5f784/php-symfony-polyfill/php-symfony-polyfill-ALL.spec

    php-composer(ircmaxell/password-compat) and php-composer(paragonie/random_compat) will also always have to be available in this case as well.

2) Provide only needed polyfills and provide conditional MACROS to consumer libraries: https://github.com/siwinski/rpms/blob/e64463828a7c70097d7f7b8463983cc570b5f784/php-symfony-polyfill/php-symfony-polyfill-MACROS.spec

    Example consumer library usage:
        %if 0%{?php_symfony_polyfill_with_php56}
        Requires: php-composer(symfony/polyfill-php56)
        %endif



I much prefer option #2 because I'd prefer not to have so many no-ops, but I'm OK with option #1 if you believe it would be better.



(In reply to Remi Collet from comment #9)
> I think it will be simpler to provide everything, whatever the PHP version
> is.
> 
> - this will be better with SCL.

SCL only provides newer versions not older versions correct?


> - this will be noop (bootstrap will be ignored)

Ignored but still processed.


> - this will avoid to have to add test in all consumer library

See macros alternate above.


> Ex: jeremeamia/superclosure requires symfony/polyfill-php56
> 
> See
> https://github.com/remicollet/remirepo/commit/
> 5ac8cde9e3b7323bb01eaba76312529495296fe8

> SCL only provides newer versions not older versions correct?

In RHEL, yes.

But in Fedora, I think is make sens to be able to install older versions for compatibility test (ex dev with 5.6, test with 5.4 for deployment on RHEL-7)

Updated to always provide all polyfills.



Spec URL: https://raw.githubusercontent.com/siwinski/rpms/17bb7c8f89e89e0a44e0ab0274e023997b06d4d2/php-symfony-polyfill/php-symfony-polyfill.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-symfony-polyfill-1.0.0-2.fc23.src.rpm

Created attachment 1103024 [details]
phpci.log

phpCompatInfo version 4.5.2 DB built Nov 25 2015 18:15:42 CET static analyze results

Created attachment 1103030 [details]
review.txt

Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04

[!]: Missing in %doc: Util/*md Util/composer.json
     and so, remove them from php dir.

[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/php/Symfony

     Must Own /usr/share/php/Symfony or require php-symfony-common


Notice, I'm ok not providing other compat layers (Iconv,Intl,Mbstring,Xml)
This only means if, from composer.json a app have 

Requires: php-composer(symfony/polyfill-intl)
Suggest:  php-intl

We'll have to use, which is fine :

Requires: php-intl

Please fix the 2 small blockers, everything else is ok.

Please also notice than https://github.com/symfony/polyfill/pull/15 is mandatory for upcomming PHPUnit 5 (I'm still blocked by PHPUnit_Selenium... but will happen...). Not mandatory for the review, we can wait for upstream merge, and apply it later, when needed.

(In reply to Remi Collet from comment #16)
> [!]: Missing in %doc: Util/*md Util/composer.json
>      and so, remove them from php dir.
> 
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/share/php/Symfony
> 
>      Must Own /usr/share/php/Symfony or require php-symfony-common
> 
> 
> Notice, I'm ok not providing other compat layers (Iconv,Intl,Mbstring,Xml)
> This only means if, from composer.json a app have 
> 
> Requires: php-composer(symfony/polyfill-intl)
> Suggest:  php-intl
> 
> We'll have to use, which is fine :
> 
> Requires: php-intl

Since RHEL's/Fedora's compiled versions of PHP include the extensions, I thought it best to require the use of the actual extension over the polyfill.



> Please fix the 2 small blockers, everything else is ok.

Fixed.  Diff = https://github.com/siwinski/rpms/commit/a25d03a83d872333a329640fd47f998026e5631c



(In reply to Remi Collet from comment #17)
> Please also notice than https://github.com/symfony/polyfill/pull/15 is
> mandatory for upcomming PHPUnit 5 (I'm still blocked by PHPUnit_Selenium...
> but will happen...). Not mandatory for the review, we can wait for upstream
> merge, and apply it later, when needed.

Based on your last comment on the pull request ("I confirm, this PR is no more required with PHPUnit 5.1.2.") I held off on adding the patch.





Spec URL: https://github.com/siwinski/rpms/blob/a25d03a83d872333a329640fd47f998026e5631c/php-symfony-polyfill/php-symfony-polyfill.spec

SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-symfony-polyfill-1.0.0-3.fc23.src.rpm

[x]: Missing in %doc: Util/*md Util/composer.json
[x]: Package must own all directories that it creates.



=== APPROVED ===

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/php-symfony-polyfill

php-symfony-polyfill-1.0.0-3.el6 php-paragonie-random-compat-1.1.0-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-b22c5c7367

php-symfony-polyfill-1.0.0-3.fc22 php-paragonie-random-compat-1.1.0-2.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-6f7a7e13f2

php-symfony-polyfill-1.0.0-3.el7 php-paragonie-random-compat-1.1.0-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-f15e95a5bd

php-symfony-polyfill-1.0.0-3.fc23 php-paragonie-random-compat-1.1.0-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-49570b2383

php-paragonie-random-compat-1.1.0-2.el7, php-symfony-polyfill-1.0.0-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'yum --enablerepo=epel-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-f15e95a5bd

php-paragonie-random-compat-1.1.0-2.el6, php-symfony-polyfill-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'yum --enablerepo=epel-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-b22c5c7367

php-paragonie-random-compat-1.1.0-2.fc22, php-symfony-polyfill-1.0.0-3.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-6f7a7e13f2

php-paragonie-random-compat-1.1.0-2.fc23, php-symfony-polyfill-1.0.0-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update php-symfony-polyfill php-paragonie-random-compat'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-49570b2383

php-paragonie-random-compat-1.1.0-2.fc23, php-symfony-polyfill-1.0.0-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

php-paragonie-random-compat-1.1.0-2.fc22, php-symfony-polyfill-1.0.0-3.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

php-paragonie-random-compat-1.1.0-2.el7, php-symfony-polyfill-1.0.0-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

php-paragonie-random-compat-1.1.0-2.el6, php-symfony-polyfill-1.0.0-3.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.