[Bug 1117022] Review Request: rubygem-combustion - Elegant Rails Engine Testing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

David Nichols <david@xxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |david@xxxxxxxx



--- Comment #1 from David Nichols <david@xxxxxxxx> ---
Hi,

Here is an informal review of the package - this is my second informal review,
so I'm hardly an expert.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

Generic:
[X]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[X]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "Unknown or generated". 6 files have unknown license. Detailed output of
     licensecheck in /export/home/dnichols/fr/1117022-rubygem-
     combustion/licensecheck.txt

Here are the contents of licensecheck.txt:

Unknown or generated
--------------------
combustion-0.5.1/lib/combustion.rb
combustion-0.5.1/lib/combustion/application.rb
combustion-0.5.1/lib/combustion/database.rb
combustion-0.5.1/lib/combustion/generator.rb
combustion-0.5.1/templates/routes.rb
combustion-0.5.1/templates/schema.rb

The package includes a LICENSE file that reflects the MIT license for the
overall package.

[!]: License file installed when any subpackage combination is installed.

You have your doc package requiring the base package, but let me quote a more
experienced reviewer about this:

"Plain documentation packages (which contain files that can be displayed with
arbitrary HTML/PDF viewers) typically do not need to depend on base libraries,
or else you could not install the documentation without pulling in dependency
bloat." (https://bugzilla.redhat.com/show_bug.cgi?id=1111691#c7)

Therefore I believe you should remove the dependency on the main package and
include %doc %{gem_instdir}/LICENCE in the %files doc section; see
http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing 
[X]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/gems,
     /usr/share/gems/doc

I assume this is OK in this case

[-]: Package contains no bundled libraries without FPC exception.
[X]: Changelog in prescribed format.
[X]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[X]: Development files must be in a -devel package
[X]: Package uses nothing in %doc for runtime.
[X]: Package consistently uses macros (instead of hard-coded directory names).
[X]: Package is named according to the Package Naming Guidelines.
[X]: Package does not generate any conflict.
[-]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[X]: Requires correct, justified where necessary.
[X]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[X]: Package is not known to require an ExcludeArch tag.
[X]: Package complies to the Packaging Guidelines

Ruby:
[X]: Platform dependent files must all go under %{gem_extdir_mri}, platform
     independent under %{gem_dir}.

Generic:
[X]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[X]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[?]: Latest version is packaged.
[X]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[X]: Package should compile and build into binary rpms on all supported
     architectures.
[?]: %check is present and all tests pass.

I assume that it's OK if there is no available no test suite:
http://fedoraproject.org/wiki/Packaging:Guidelines#Test_Suites

[X]: Packages should try to preserve timestamps of original installed files.

[X]: Gem should use %gem_install macro.

I don't know why fedora-review reported this as failed - the spec file does use
it.

[?]: Test suite of the library should be run.

as per above

[?]: Specfile should use macros from rubygem-devel package.
     Note: The specfile doesn't use these macros: %exclude %{gem_cache},
     %{gem_spec}

These are included in the specfile, so I'm not sure what fedora-review is
reporting here.

There is only one rpmlint warning:
rubygem-combustion.noarch: W: no-manual-page-for-binary combust

I assume this is not a blocker.

Note that I only included fedora-review items above where the result was not OK
or incorrect from my point of view in the fedora-review output.

Hope this helps,
David

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]