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=542559 --- Comment #2 from Matthew Kent <mkent@xxxxxxxxxxxx> 2009-12-21 00:12:21 EDT --- Thank you for the review. (In reply to comment #1) > Some notes: > > * Source > - Source0 in srpm differs from what I could download from > the URL written in the spec file > ------------------------------------------------------------- > 5fa79d6ca562a39c72c89f5447a3fbd5 thor-0.12.0.gem > 32e034949be3726ff1857d0edeae6566 > rubygem-thor-0.12.0-1.fc13.src/thor-0.12.0.gem > ------------------------------------------------------------- > (and the contents of two gems actually differ) > Good catch! Looks like they replaced it after the recent rubyforge -> gemcutter migration. > * Requires > - bin/rake2thor contains: > ------------------------------------------------------------- > 8 require 'rake' > ------------------------------------------------------------- > As the Summary of this spec file says "that replaces rake,", > I think it is admitted to add "Requires: rubygem(rake)" and > this should surely be added. > Yeah I went back and forth on this one a bit initial, but your right, it should be there. > - lib/thor/shell/color.rb contains: > ------------------------------------------------------------- > 98 @diff_lcs_loaded = begin > 99 require 'diff/lcs' > 100 true > 101 rescue LoadError > 102 false > 103 end > ------------------------------------------------------------- > I guess "diff/lcs" dependency is surely optional, however > as Fedora already has rubygem-diff-lcs, you may want to > add this dependency (however this is up to what you think) Oh, missed this one. Looks like a minimal impact to add it as diff-lcs doesn't have any dependencies of its own. -- 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