Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: rubygem-multi_xml - A generic swappable back-end for XML parsing
https://bugzilla.redhat.com/show_bug.cgi?id=785639
Summary: Review Request: rubygem-multi_xml - A generic swappable back-end for XML parsing Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: mastahnke@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
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.r... 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)
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=785639
Michael Stahnke mastahnke@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |781829
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=785639
Bohuslav Kabrda bkabrda@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |bkabrda@redhat.com AssignedTo|nobody@fedoraproject.org |bkabrda@redhat.com Flag| |fedora-review?
--- Comment #1 from Bohuslav Kabrda bkabrda@redhat.com 2012-01-30 03:31:10 EST --- I'll take this for a review.
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=785639
--- Comment #2 from Bohuslav Kabrda bkabrda@redhat.com 2012-01-30 03:55:24 EST --- - 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.)
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=785639
--- Comment #3 from Michael Stahnke mastahnke@gmail.com 2012-02-21 00:01:24 EST --- Updated SRPMS at http://stahnma.fedorapeople.org/reviews/rubygem-multi_xml-0.4.1-2.fc17.src.r... 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.
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=785639
--- Comment #4 from Bohuslav "Slavek" Kabrda bkabrda@redhat.com 2012-02-21 03:01:38 EST --- 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.
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=785639
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vondruch@redhat.com
--- Comment #5 from Vít Ondruch vondruch@redhat.com 2012-03-06 06:44:41 EST --- 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.
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=785639
Bohuslav "Slavek" Kabrda bkabrda@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #6 from Bohuslav "Slavek" Kabrda bkabrda@redhat.com 2012-03-06 08:25:38 EST --- 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.
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=785639
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #7 from Vít Ondruch vondruch@redhat.com 2012-03-06 09:20:21 EST --- 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:
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=785639
--- Comment #8 from Jon Ciesla limburgher@gmail.com 2012-03-06 09:26:51 EST --- Git done (by process-git-requests).
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=785639
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
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=785639
--- Comment #9 from Fedora Update System updates@fedoraproject.org 2012-03-06 10:02:30 EST --- 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
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=785639
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #10 from Fedora Update System updates@fedoraproject.org 2012-03-07 02:23:23 EST --- 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... then log in and leave karma (feedback).
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=785639
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |rubygem-multi_xml-0.4.1-3.f | |c17 Resolution| |ERRATA Last Closed| |2012-03-11 21:43:23
--- Comment #11 from Fedora Update System updates@fedoraproject.org 2012-03-11 21:43:23 EDT --- 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.
package-review@lists.fedoraproject.org