https://bugzilla.redhat.com/show_bug.cgi?id=2247205

Xavier Bachelot xavier@bachelot.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Comment|0 |updated

--- Comment #0 has been edited ---

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

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

Remi Collet fedora@famillecollet.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Doc Type|--- |If docs needed, set a value CC| |fedora@famillecollet.com

--- Comment #2 from Remi Collet fedora@famillecollet.com --- 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)

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

--- Comment #4 from Xavier Bachelot xavier@bachelot.org --- 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.rp...

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

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

--- Comment #6 from Remi Collet fedora@famillecollet.com --- Created attachment 2001264 --> https://bugzilla.redhat.com/attachment.cgi?id=2001264&action=edit review.txt

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

--- Comment #8 from Remi Collet fedora@famillecollet.com --- 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)

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

--- Comment #9 from Xavier Bachelot xavier@bachelot.org --- Thanks for the review Remi :-) I'll see what I can do to help with your reviews asap.

https://bugzilla.redhat.com/show_bug.cgi?id=2247205

Xavier Bachelot xavier@bachelot.org changed:

What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |RAWHIDE Status|POST |CLOSED Last Closed| |2024-02-06 09:36:45