Bug 237380 (ruby-gettext-package)
Summary: | Review Request: ruby-gettext-package - Localization Library and Tools for Ruby | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mamoru TASAKA <mtasaka> |
Component: | Package Review | Assignee: | Chitlesh GOORAH <chitlesh> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | Flags: | chitlesh:
fedora-review+
wtogami: fedora-cvs+ |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-05-12 03:32:48 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 237382 |
Description
Mamoru TASAKA
2007-04-21 17:03:29 UTC
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 (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 --------------------------------------------- Any issues left on this package? Same as ruby-amazon, the folders samples and test were omitted. Their contents should be added to one or more sub packages. 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.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. 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 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.u-tokyo.ac.jp Branches: devel FC-6 FC-5 InitialCC: (nobody) ----------------------------------------------- Rebuild done on all branches, closing Thank you for review!! |