[Bug 477883] Review Request: rubygem-nokogiri - An HTML, XML, SAX, and Reader parser

[bugzilla at redhat.com]

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=477883





--- Comment #3 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-01-17 09:57:22 EDT ---
Thank you for initial comments.

(In reply to comment #2)
> No license file is included in the .gem. Can you ask the upstream to put a
> COPYING or LICENSE file into the gem? Not a negative on the review, but a
> suggestion for upstream if you could pass it along.

- The license clause is included in README{,.ja}.txt.

> In the spec the file hoe.rb is deleted. Is this package dependant on hoe? You
> should add rubygem(hoe) as a dependency.

- No. This is (hoe.rb) only needed for Rakefile and so not needed at
  runtime.

> The install target directory used is the relative path "./" rather than
> %{buildroot} macro. Please fix that.

- In this case (i.e. in the case rubygem creates C module)
  the way of packaging gem into rpm is different. Actually I wrote
  the packaging draft and it was accepted by FPC/FESCo. Please refer to
  my first comment on this bug.

> 
> Perhaps I'm misreading, but I don't see where the RPM claims to own
> %{geminstdir}, instead it just lists the contents of that directory. Can you
> make it more explicit?

- Well, I already write this explicitly.
-------------------------------------------------------------
%files
%defattr(-,root, root,-)
%{ruby_sitearch}/%{gemname}/
%dir    %{geminstdir}/ <-------------------------------
%doc    %{geminstdir}/[A-Z]*
%{geminstdir}/[a-l]*/
-------------------------------------------------------------


> For the subpackage ruby-nokogiri, the Requires: does not match the packaging
> guidelines: the guideline says the non-gem should require "rubygem(%{name})",
> but the spec file has "%{name}". Please fix that.

- "Requires %{name} = %{version}-%{release}" will pull in
  rubygem(%{gemname}), and this dependency is more strict than
  "Requires: rubygem(%{gemname})", so this should be okay.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.