Spec URL: https://www.bachelot.org/fedora/SPECS/php-smarty-gettext.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/php-smarty-gettext-1.7.0-1.fc40.src.rpm Description: smarty-gettext provides gettext (i18n) support for Smarty, the popular PHP templating engine, to implement an NLS (Native Language Support) API which can be used to internationalize and translate your PHP applications. Fedora Account System Username: xavierb
Copr build: https://copr.fedorainfracloud.org/coprs/build/6584134 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2247205-php-smarty-gettext/fedora-rawhide-x86_64/06584134-php-smarty-gettext/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Comments (not yet a review) Have you tried to run the upstream test suite? OK PHPUnit 4 is terribly old, and removed from Fedora... but a small patch should allow it to run with a higher version I was able to run it using PHPUnit 9 (using PHP 7.4, 8.0, 8.1, 8.2 and 8.3) with a simple - public static function setUpBeforeClass(){ + public static function setUpBeforeClass():void{ You also need to have BuildRequires: langpacks-pl langpacks-et langpacks-en Will be nice to provide an autoloader, for app using this library a simple "require" of the 2 files especially as more files may appear in the future Perhaps /usr/share/php/Smarty/plugins/gettext-autoload.php ? But, what will happen if multiple plugins are packaged/installed? Perhaps a better layout may be /usr/share/php/Smarty/plugins/ /usr/share/php/Smarty/plugins/gettext /usr/share/php/Smarty/plugins/gettext/autoload.php /usr/share/php/Smarty/plugins/gettext/block.t.php /usr/share/php/Smarty/plugins/gettext/function.locale.php Spec has: Requires: php-Smarty As composer.json has "smarty/smarty": "3.1.*" So this should be Requires: (php-composer(smarty/smarty) >= 3.1 with php-composer(smarty/smarty) < 4) Composer.json has "license": "LGPL-2.1", Spec has License: LGPL-2.1-or-later README.md has "either version 2.1 of the License, or (at your option) any later version." According to the readme file, you're right, so needs to be reported upstream to fix the SPDX ID in composer.json Spec has: Requires: php-pcre pcre extension is not required by packaged files (only by tsmarty2c.php which is not installed)
Notice for the test suite, you need azatoth/php-pgettext which is not packaged so you can either - skip MsgctxtTest::translateTest - include pgettext.php as sources (acceptable as only used at build time, and seems a dead project)
Spec URL: https://www.bachelot.org/fedora/SPECS/php-smarty-gettext.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/php-smarty-gettext-1.7.0-2.fc40.src.rpm Changes: - Provide autoloader - Run test suite MsgctxtTest is currently skipped, I'll get back to that. I have not added the constraint on Smarty < 4, as it is actually supported. See https://github.com/smarty-gettext/smarty-gettext/issues/42 Also, I have not added a subdirectory for gettext, as upstream instruct to put plugins directly under the plugins directory. https://www.smarty.net/docs/en/variable.plugins.dir.tpl
Copr build: https://copr.fedorainfracloud.org/coprs/build/6651072 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2247205-php-smarty-gettext/srpm-builds/06651072/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Created attachment 2001264 [details] review.txt
Package complies to the Packaging Guidelines *** APPROVED ***
Sorry for the delay, quite busy with 8.3.0 Feel free to pick one of https://blog.remirepo.net/pages/Pending-reviews (especially php-nikic-php-parser5 needed for 8.3)
Thanks for the review Remi :-) I'll see what I can do to help with your reviews asap.
The Pagure repository was created at https://src.fedoraproject.org/rpms/php-smarty-gettext