Bug 237380 - (ruby-gettext-package) Review Request: ruby-gettext-package - Localization Library and Tools for Ruby
Review Request: ruby-gettext-package - Localization Library and Tools for Ruby
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chitlesh GOORAH
Fedora Package Reviews List
:
Depends On:
Blocks: alexandria
  Show dependency treegraph
 
Reported: 2007-04-21 13:03 EDT by Mamoru TASAKA
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-11 23:32:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chitlesh: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mamoru TASAKA 2007-04-21 13:03:29 EDT
Spec URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/ruby-gettext-package.spec
SRPM URL: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/ruby-gettext-package-1.9.0-1.fc7.src.rpm
Mock build log on FC-devel i386: http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/LOGS/MOCK-ruby-gettext-package.log

Description: 
Ruby-GetText-Package is a Localization(L10n) Library and Tools 
which is modeled after GNU gettext package.

The library converts the messages to localized messages properly 
using client-side locale information. And the tools for developers 
support to create, use, and modify localized message files
(message catalogs) easily.
Comment 1 Jasper O. Hartline 2007-05-01 10:58:39 EDT
Chitlesh Goorah has asked me to do an informal review of this package.
Upon installing all BuildRequires, the package builds cleanly in Fedora Core 6
with a single warning from rpmlint:
W: ruby-gettext-package invalid-license Ruby License/LGPL

This should be LGPL rather than Ruby License/LGPL

In examining your SPEC file, I see you can collapse some Requires and
BuildRequires using this format:
BuildRequires:  irb, ruby-devel, ruby(abi = %{rubyabi}
Requires:       irb, ruby(abi) = %{rubyabi}

rather than:
BuildRequires:  irb
BuildRequires:  ruby-devel
BuildRequires:  ruby(abi) = %{rubyabi}
Requires:       ruby(abi) = %{rubyabi}
Requires:       irb

I also notice in the SPEC, the %files section has:
%files -f %{name}.lang
You might want to look into using:
%{find_lang} 
instead.

See: rpmbuild --showrc | grep find_lang

Comment 2 Mamoru TASAKA 2007-05-01 11:09:27 EDT
(In reply to comment #1)
> with a single warning from rpmlint:
> W: ruby-gettext-package invalid-license Ruby License/LGPL
> 
> This should be LGPL rather than Ruby License/LGPL
See the real COPYING text. This is licensed under
LGPL or Ruby License. This type of dual license is very
common for ruby related modules

> 
> In examining your SPEC file, I see you can collapse some Requires and
> BuildRequires using this format:
> BuildRequires:  irb, ruby-devel, ruby(abi = %{rubyabi}
> Requires:       irb, ruby(abi) = %{rubyabi}
> 
> rather than:
> BuildRequires:  irb
> BuildRequires:  ruby-devel
> BuildRequires:  ruby(abi) = %{rubyabi}
> Requires:       ruby(abi) = %{rubyabi}
> Requires:       irb

I _strongly_ recommend the latter style (i.e. my style) and
also some other reviewers recommend the latter style
* This style makes it easy that what is really changed on
  taking diff when the dependency is changed.
* This style makes diff output smaller.
* For this package, the BuildRequires has only 3 packages so
  the difference is small.
  However, please consider the case in which one package has
  29 BuildRequires......

> I also notice in the SPEC, the %files section has:
> %files -f %{name}.lang
> You might want to look into using:
> %{find_lang} 
> instead.

Already I use %find_lang. Note that there is no 
%{name}.mo files
--------------------------------------------
%find_lang rails
%find_lang rgettext
%{__cat} *.lang >> %{name}.lang
---------------------------------------------
Comment 3 Mamoru TASAKA 2007-05-05 14:19:34 EDT
Any issues left on this package?
Comment 4 Chitlesh GOORAH 2007-05-06 12:40:02 EDT
Same as ruby-amazon, the folders samples and test were omitted. Their contents 
should be added to one or more sub packages.
Comment 5 Mamoru TASAKA 2007-05-06 14:24:55 EDT
Well:

http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SPECS/ruby-gettext-package.spec
http://www.ioa.s.u-tokyo.ac.jp/~mtasaka/dist/extras/development/SRPMS/ruby-gettext-package-1.9.0-2.fc7.src.rpm
------------------------------------------------
* Mon May  7 2007 Mamoru Tasaka <mtasaka@ioa.s.u-tokyo.ac.jp> - 1.9.0-2
- Create -doc subpackage

For rpmlint:
------------------------------------------------
1 W: ruby-gettext-package invalid-license Ruby License/LGPL
1 W: ruby-gettext-package-debuginfo invalid-license Ruby License/LGPL
2 E: ruby-gettext-package-doc zero-length
/usr/share/doc/ruby-gettext-package-doc-1.9.0/samples/rails/public/favicon.ico
3 W: ruby-gettext-package-doc spurious-executable-perm
/usr/share/doc/ruby-gettext-package-doc-1.9.0/test/test.sh
1 W: ruby-gettext-package-doc invalid-license Ruby License/LGPL
------------------------------------------------
1: This type of dual license is very common for ruby related
   software
2: Well, I am not familiar with "ruby on rails" and I am not sure
   if I should (can) remove this file or not......
3: This is okay because this script is actually executable and
   only /bin/sh dependency is added to -doc subpackage due to
   this permission.
Comment 6 Chitlesh GOORAH 2007-05-10 14:32:30 EDT
MUST Items:

- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed with an open-source compatible license 
and meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own 
file, then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible. 
- MUST: The sources used to build the package must matches the upstream 
source, as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i386.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.
- MUST: If the package does not contain shared library files located in the
dynamic linker's default paths
- MUST: the package is not designed to be relocatable
- MUST: the package owns all directories that it creates.
- MUST: the package does not contain any duplicate files in the %files 
listing.
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot} 
(or
$RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros 
section of Packaging Guidelines.
- MUST: The package contains code, or permissable content. This is described 
in detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If 
it is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package containing GUI applications includes a %{name}.desktop file, 
and that file must be properly installed with desktop-file-install in 
the %install section.
- MUST: Package does not own files or directories already owned by other 
packages. 

SHOULD Items:

 - SHOULD: The source package does include license text(s) as COPYING
 - SHOULD: mock builds successfully in i386.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD: No scriptlets were used, those scriptlets must be sane. 
 - SHOULD: No subpackages present.

APPROVED
Comment 7 Mamoru TASAKA 2007-05-10 14:36:43 EDT
Thank you!!

Request for CVS admin:
-----------------------------------------------
New Package CVS Request
=======================
Package Name:         ruby-gettext-package
Short Description:    Localization Library and Tools for Ruby
Owners:               mtasaka@ioa.s.u-tokyo.ac.jp
Branches:             devel FC-6 FC-5
InitialCC:            (nobody)
-----------------------------------------------
Comment 8 Mamoru TASAKA 2007-05-11 23:32:48 EDT
Rebuild done on all branches, closing

Thank you for review!!

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