This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 701028

Summary: Review Request: php-Smarty-gettext - Gettext support for Smarty
Product: [Fedora] Fedora Reporter: Olivier BONHOMME <obonhomme>
Component: Package ReviewAssignee: Matthias Runge <mrunge>
Status: CLOSED INSUFFICIENT_DATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: fedora, fedora-package-review, mrunge, notting, obonhomme, pahan
Target Milestone: ---Flags: mrunge: fedora‑review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-09-24 04:38:10 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 704672    

Description Olivier BONHOMME 2011-04-30 09:01:18 EDT
Spec URL:  http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext.spec
SRPM URL: http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext-1.0b1-1.fc14.src.rpm
Description: Smarty gettext plug-in provides an internationalization support
for the PHP template engine Smarty.

Hello, I submit this Review Request to the Fedora Package team in order to integrate this library in the next Fedora Release. It is my first package, so I request a sponsor for my first review process.

Thanks a lot.
Comment 1 Remi Collet 2011-05-14 04:25:31 EDT
Some notes (not a review)


- version is not correct for a pre-release version (1.0-0.1.b1)
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

- use a simple sed instead of dos2unix (avoid dependency)

- don't install doc, list them in %file is enough
%doc README COPYING Changelog

- what is the goal of the 01_fix_binary_license.patch ?
only to change "space"  (diff -b is empty) ?
I don't think it is really usefull... 

- name is not correct
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28General.29
and http://fedoraproject.org/wiki/Packaging/PHP#Naming_scheme

- removing .php suffix from installed binary seems a good idea 

- shebang must be fixed ("/usr/bin/env php" => %{_bindir}/php)
Comment 2 Olivier BONHOMME 2011-05-14 09:58:07 EDT
Hello,

Thanks for your comments. I considered all your remarks and made new spec file. You can find : 

- The SPEC file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext.spec
- The SRPM file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.1.b1.fc14.src.rpm

Regards,
Comment 3 Remi Collet 2011-05-15 11:51:37 EDT
- spec file name must be fixed
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Spec_file_name

- use install with -p option to preserve file timestamp

- sed are unreadable, use special substitution instead

- should preserve timestamp on upstream as much as possible

per exemple : 
# UNIX format conversion
for fic in ChangeLog README COPYING; do
  touch -r $fic .timestamp
  sed -i -e 's/\f//;s/\r//' $fic
  touch -r .timestamp $fic
done

- remove rm -rf %{buildroot} in %prep
$ rpmlint  -I rpm-buildroot-usage
rpm-buildroot-usage:
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it may
break short circuit builds.

-add rm -rf %{buildroot} in %install

- issue with license file
$ rpmlint -I incorrect-fsf-address
incorrect-fsf-address:
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

You should, at least, open an upstream bug (don't know if he's alive, as this 1.0b1 seems quite old... 2005)
Comment 4 Olivier BONHOMME 2011-05-15 18:06:19 EDT
Hello,

Here is a new version considering the last notes : 

- SPEC file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.1.b1.fc14.src.rpm

Regards,
Comment 5 Olivier BONHOMME 2011-07-03 17:41:10 EDT
Hello,

I have made a new version considering the new recommendations provided by the FC15 rpmlint software.

- SPEC file :
http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM file :
http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.2.b1.fc15.src.rpm

Regards,
Comment 6 Olivier BONHOMME 2011-12-01 12:16:12 EST
Hello,

I just repackaged the php-Smarty-gettext library for FC16. You'll find the elements here : 

- SPEC File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.2.b1.fc16.src.rpm

Regards,
Comment 7 Olivier BONHOMME 2012-08-23 08:56:48 EDT
Hello,

Here is the new RPM for the smarty gettext library rebuilt for Fedora 17. The main change is the manpage add for the tsmarty2c script.

The ressources are here : 

- SPEC File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.3.b1.fc17.src.rpm.

Moreover, since the project is inactive since years, it is planned to fork or recover the upstream project in order to import patches and updates.

Regards,
Comment 8 Matthias Runge 2013-05-14 06:41:15 EDT
Olivier, are you still around and interested?
Comment 9 Olivier BONHOMME 2013-05-14 07:12:39 EDT
Hello Matthias,

Of course I am. The smarty-gettext lib is still used by our project Fusiondirectory which is still looking for a sponsor.

Regards,
Olivier
Comment 10 Matthias Runge 2013-05-14 07:17:15 EDT
OK, I'll do a review, and also could sponsor you.
Comment 11 Matthias Runge 2013-05-14 07:33:20 EDT
some drive-by comments:
- please drop buildroot definition, it's not required any more
- please drop provides and obsoletes, esp. the latter doesn't make sense at all here
- please remove the clean section, not required any more
- please update to latest release.
- please drop defattr in files section, not required any more
Comment 12 Matthias Runge 2013-08-26 04:55:22 EDT
Any progress here?
Comment 13 Matthias Runge 2013-09-24 04:38:10 EDT
Closing this. Please open another one, when you're ready.