Spec URL: http://stahnma.fedorapeople.org/reviews/rubygem-multi_xml.spec SRPM URL: http://stahnma.fedorapeople.org/reviews/rubygem-multi_xml-0.4.1-1.fc17.src.rpm Description: A gem to provide swappable XML backends utilizing LibXML, Nokogiri, Ox, or REXML. Notes: This is built against ruby 1.8 (I need it in F16 to fix bug 781829)
I'll take this for a review.
- Run the specs -- Add BR: rubygem(rspec-core) and BR: rubygem(nokogiri) -- Comment out/delete the first two lines in spec/helper.rb to avoid the unnecessary simplecov dependency. -- Run the tests like "rspec spec" in %geminstdir - note that you will probably need to add some load paths later with Ruby 1.9.3. - Other than that, the package looks fine. I would maybe consider using %exclude in %files section rather than removing files with "rm" in %install, but this is definitely not a blocker. So please run the rspecs and then this package can be approved. (When building for rawhide, please save yourself some work and build it right into the f17-ruby target, it will be merged into rawhide anyway.)
Updated SRPMS at http://stahnma.fedorapeople.org/reviews/rubygem-multi_xml-0.4.1-2.fc17.src.rpm Updated SPEC at http://stahnma.fedorapeople.org/reviews/rubygem-multi_xml.spec I wouldn't mind a look over as I'm trying to use the same specs for EPEL and Fedora, so I have some 1.8 vs 1.9 stuff in there.
First of all, this spec doesn't work on F17 at all. - Please use the new style macros (like gem_dir instead of gemdir etc.). - Then define the macros properly - %(ruby -rubygems -e 'puts Gem::dir' 2>/dev/null) now points to user's home. I suggest using some kind of conditionals like this one: %{!?gem_dir: %global gem_dir %(ruby -rubygems -e 'puts Gem::dir' 2>/dev/null)} This way, if the gem_dir macro is defined in rubygems-devel, it will get used (f17 or later), otherwise the former directories (f16 and before) will get in place. Similarly for all the other macros (and you need to define gem_name, not gemname in order for the macros in rubygems-devel to work properly). - In Ruby 1.9.3, we have unbundled rubygem-bigdecimal, so in the "f17 or higher" conditional, you need to add R: and BR: rubygem(bigdecimal). Now some general comments: - Please don't run the specs through rake. It draws in unnecessary dependencies like yard (or the rake itself). Simple "rspec spec" suffices. - Don't run tests in buildroot, we don't want to modify the files in resulting package by sed just because we needed to run the tests in some particular way. Instead, run the tests in BUILD (therefore in %check, do "pushd .%{gem_instdir}). - Also, by not running the tests via rake, you just need to delete/comment out the first two lines of spec/helper.rb and the tests run (you can do this in the %check section (therefore in BUILD directory)). - According to the new quidelines draft, you should exclude the cached version of gem.
Hi, Since this is pretty much blocking several packages, I prepared updated package. I hope you don't mind. Spec URL: http://people.redhat.com/vondruch/rubygem-multi_xml.spec SRPM URL: http://people.redhat.com/vondruch/rubygem-multi_xml-0.4.1-3.fc18.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3857549 Please note that this package is just for Fedora yet, since in EPEL, there is no rspec-core, no rubygems-devel and it would need more work.
This package looks very good. I would maybe consider moving %doc %{gem_instdir}/README.md to -doc subpackage, but this is really a minority. This package is APPROVED.
Thank you for your review. Michael, I put you as a co-maintainer if you care about older Fedoras and EPEL. New Package SCM Request ======================= Package Name: rubygem-multi_xml Short Description: A generic swappable back-end for XML parsing Owners: vondruch bkabrda stahnma Branches: f17 InitialCC:
Git done (by process-git-requests).
rubygem-multi_xml-0.4.1-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/rubygem-multi_xml-0.4.1-3.fc17
Package rubygem-multi_xml-0.4.1-3.fc17: * should fix your issue, * was pushed to the Fedora 17 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing rubygem-multi_xml-0.4.1-3.fc17' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2012-3213/rubygem-multi_xml-0.4.1-3.fc17 then log in and leave karma (feedback).
rubygem-multi_xml-0.4.1-3.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.