Bug 701028 - Review Request: php-Smarty-gettext - Gettext support for Smarty
Summary: Review Request: php-Smarty-gettext - Gettext support for Smarty
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Matthias Runge
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 704672
TreeView+ depends on / blocked
 
Reported: 2011-04-30 13:01 UTC by Olivier BONHOMME
Modified: 2013-10-19 14:42 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-09-24 08:38:10 UTC
Type: ---
Embargoed:
mrunge: fedora-review?


Attachments (Terms of Use)

Description Olivier BONHOMME 2011-04-30 13:01:18 UTC
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 08:25:31 UTC
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 13:58:07 UTC
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 15:51:37 UTC
- 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 22:06:19 UTC
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 21:41:10 UTC
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 17:16:12 UTC
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 12:56:48 UTC
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 10:41:15 UTC
Olivier, are you still around and interested?

Comment 9 Olivier BONHOMME 2013-05-14 11:12:39 UTC
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 11:17:15 UTC
OK, I'll do a review, and also could sponsor you.

Comment 11 Matthias Runge 2013-05-14 11:33:20 UTC
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 08:55:22 UTC
Any progress here?

Comment 13 Matthias Runge 2013-09-24 08:38:10 UTC
Closing this. Please open another one, when you're ready.


Note You need to log in before you can comment on or make changes to this bug.