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=541491 --- Comment #2 from Matthew Kent <mkent@xxxxxxxxxxxx> 2009-11-30 01:51:49 EDT --- Thank you for the review. (In reply to comment #1) > Some notes: > > * Explicit version dependency > - ">= 3.0" on Requires: rubygem(sexp_processor) is redundant > because all rubygem-sexp_processor shipped on Fedora satisfies > this version dependency: > https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires > Noted, thanks. > * %check > ---------------------------------------------------------------- > 16 # These test cases are carried in the ParseTree gem in test/. Carry > them here > 17 # rather than attempting to install ParseTree-doc in check and > introducing a circular > 18 # dependency > 19 Source1: pt_testcase.rb > 79 %check > 80 pushd .%{geminstdir} > 81 cp %{SOURCE1} test/ > 82 rake test > ---------------------------------------------------------------- > - IMO if this script is really needed for "rake test" (and actually > it seems so), this script should also be included in the rebuilt > binary rpm (i.e. better to move the lines 80-81 to %build). > Good idea. > ? Dependency loop > - lib/gauntlet_rubyparser.rb contains: > ---------------------------------------------------------------- > 8 require 'rubygems' > 9 require 'ruby2ruby' > 10 require 'ruby_parser' > 11 > 12 require 'gauntlet' > ---------------------------------------------------------------- > i.e. this script needs two other gems: "ruby2ruby" "gauntlet" > - The formar one causes dependency loop > - The latter one is not found on Fedora (even on review request) > Can this dependency (rather, this script) be ignored? Ah you noticed this as well, I should have added some notes inline. I chose to ignore it's dependencies as the developer didn't add it as either a primary or development dependency in the Rakefile. It's also not a library as far as I can tell, more of a script used in testing, and not included/invoked in any of the unit testing. Plus looking at the gauntlet gem itself it's clearly geared toward library development rather than providing functionality to a library. What's the correct approach here, %exclude the script? Add a note about it and leave dependencies as is? Will go on the assumption you'd rather it be excluded. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review