Spec URL: https://raw.githubusercontent.com/siwinski/rpms/a4794fbacbe6a03f5a9fbabdd1af53f5820648a1/php-simplesamlphp-saml2/php-simplesamlphp-saml2.spec SRPM URL: https://siwinski.fedorapeople.org/SRPMS/php-simplesamlphp-saml2-2.2-1.fc24.src.rpm Description: A PHP library for SAML2 related functionality. Extracted from SimpleSAMLphp [1], used by OpenConext [2]. This library started as a collaboration between UNINETT [3] and SURFnet [4] but everyone is invited to contribute. Autoloader: %{phpdir}/SAML2/autoload.php [1] https://www.simplesamlphp.org/ [2] https://www.openconext.org/ [3] https://www.uninett.no/ [4] https://www.surfnet.nl/ Fedora Account System Username: siwinski Copr build: https://copr.fedorainfracloud.org/coprs/siwinski/simplesamlphp/package/php-simplesamlphp-saml2/
Created attachment 1185557 [details] phpci.log phpCompatInfo version 5.0.1 DB version 1.11.0 built Jul 26 2016 06:33:42 CEST static analyze results
Created attachment 1185558 [details] review.txt Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -b 1356587 Buildroot used: fedora-rawhide-x86_64
Everything looks ok (from packaging PoV), no Blockers Only one clarification: IIUC, _autoload.php is designed for compatibility with non-namespaced version 1 of the library. So application using this library probably don't need it. Instead of including _autoload.php (without NS) from autoload.php (SAML2 NS) shouldn't it be better to do the opposite ? application using v1 will include _autoload.php, thus will get a working autoloader without NS (and SAML2 NS required by class alias). application using v2 will include autoload.php, this will get a working autoloader "only" for SAML2 NS. What do you think ?
(In reply to Remi Collet from comment #3) > Everything looks ok (from packaging PoV), no Blockers > > > Only one clarification: > > IIUC, _autoload.php is designed for compatibility with non-namespaced > version 1 of the library. > > So application using this library probably don't need it. > > Instead of including _autoload.php (without NS) from autoload.php (SAML2 NS) > shouldn't it be better to do the opposite ? > > application using v1 will include _autoload.php, thus will get a working > autoloader without NS (and SAML2 NS required by class alias). > > application using v2 will include autoload.php, this will get a working > autoloader "only" for SAML2 NS. > > What do you think ? I don't like that `_autoload.php` states "Temporary autoloader". I do not want to be responsible for making sure that file is correct and v1 compatibility always works if just packaging v2. I would much rather have the two separate pkgs and just keep them updated to upstream's separate releases. If you look at upstream releases [1], 1.9 was released after 2.2. Even though right now v2 might be suitable as a drop-in replacement for v1, I'd rather not be responsible for that. [1] https://github.com/simplesamlphp/saml2/releases
> I would much rather have the two separate pkgs Ok, so just don't package this _autoload.php ;)
(In reply to Remi Collet from comment #5) > > I would much rather have the two separate pkgs > > Ok, so just don't package this _autoload.php ;) OK, I'll remove it after initial import THANKS for the review! SCM request submitted via pkgdb.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/php-simplesamlphp-saml2
php-simplesamlphp-saml2-2.2-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-9c4a3a8b05
php-simplesamlphp-saml2-2.2-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-9a44649dc6
php-simplesamlphp-saml2-2.2-2.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-6288c4728e
php-simplesamlphp-saml2-2.2-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6e6d8a3b98
php-simplesamlphp-saml2-2.2-2.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-9c4a3a8b05
php-simplesamlphp-saml2-2.2-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-6e6d8a3b98
php-simplesamlphp-saml2-2.2-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-6288c4728e
php-simplesamlphp-saml2-2.2-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-9a44649dc6
php-simplesamlphp-saml2-2.2-2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.
php-simplesamlphp-saml2-2.2-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
php-simplesamlphp-saml2-2.2-2.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-simplesamlphp-saml2-2.2-2.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.