[Bug 785639] Review Request: rubygem-multi_xml - A generic swappable back-end for XML parsing

bugzilla at redhat.com bugzilla at redhat.com
Tue Feb 21 08:01:39 UTC 2012


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 at 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.

-- 
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.



More information about the package-review mailing list