Bug 477883
| Summary: | Review Request: rubygem-nokogiri - An HTML, XML, SAX, and Reader parser | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Mamoru TASAKA <mtasaka> |
| Component: | Package Review | Assignee: | Darryl L. Pierce <dpierce> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | dpierce, fedora-package-review, jstribny, notting, tremble, tross |
| Target Milestone: | --- | Flags: | dpierce:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-01-20 05:30:21 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: | 477533 | ||
|
Description
Mamoru TASAKA
2008-12-24 18:32:27 UTC
http://mtasaka.fedorapeople.org/Review_request/rubygem-nokogiri/rubygem-nokogiri.spec http://mtasaka.fedorapeople.org/Review_request/rubygem-nokogiri/rubygem-nokogiri-1.1.1-1.fc.src.rpm * Thu Jan 15 2009 Mamoru Tasaka <mtasaka.u-tokyo.ac.jp> - 1.1.1-1 - 1.1.1 For F-11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1055417 For F-10: http://koji.fedoraproject.org/koji/taskinfo?taskID=1055424 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.
In the spec the file hoe.rb is deleted. Is this package dependant on hoe? You should add rubygem(hoe) as a dependency.
The install target directory used is the relative path "./" rather than %{buildroot} macro. Please fix that.
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?
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.
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. APPROVED by Darryl Pierce (dpierce) Thank you! New Package CVS Request ======================= Package Name: rubygem-nokogiri Short Description: An HTML, XML, SAX, and Reader parser Owners: mtasaka Branches: F-10 F-9 InitialCC: (nobody) cvs done. Rebuilt on all branches, submitted push request for F-10/9, closing. Thank you for reviewing and CVS procedure. Package Change Request ====================== Package Name: rubygem-nokogiri New Branches: EL-4 EL-5 Owners: tremble @ Mark It seems that you are not in Fedora's packager group. Any person who wants to maintain packages on Fedora/EPEL must be in this group and to get yourself added in package group, you must get sponsored first: http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored To CVS admins: Mark is now sponsored by me. As I have no intention to maintain packages on EPEL, I am happy with that Mark will maintain this on EPEL. cvs done. Package Change Request ====================== Package Name: rubygem-nokogiri New Branches: epel7 Owners: jstribny humaton Branch exist. |