[Bug 701028] New: Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: smarty-gettext -  Gettext support for Smarty

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

Summary: Review Request: smarty-gettext -  Gettext support for
                    Smarty
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: unspecified
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora
      Story Points: ---

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.0b...
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.

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Olivier BONHOMME  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |177841(FE-NEEDSPONSOR)

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Remi Collet  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from 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_package...

- 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_.28G...
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)

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #2 from 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-...

Regards,

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #3 from 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)

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #4 from 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....
- SRPM file :
http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-...

Regards,

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #5 from 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....
- SRPM file :
http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-...

Regards,

[Bug 701028] Review Request: smarty-gettext - Gettext support for Smarty

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #6 from 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....
- SRPM File :
http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-...

Regards,