https://bugzilla.redhat.com/show_bug.cgi?id=2247205
Bug ID: 2247205 Summary: Review Request: php-smarty-gettext - Gettext support for Smarty Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: xavier@bachelot.org QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
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.<description here> Fedora Account System Username: xavierb
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
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/smarty-g | |ettext/smarty-gettext
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6584134 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
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 #3 from Remi Collet fedora@famillecollet.com --- 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)
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 #5 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6651072 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
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
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+ Status|NEW |POST
--- Comment #7 from Remi Collet fedora@famillecollet.com --- Package complies to the Packaging Guidelines
*** APPROVED ***
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
Xavier Bachelot xavier@bachelot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@famillecollet.com
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
--- Comment #10 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/php-smarty-gettext
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
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com