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=543549 --- Comment #4 from Michal Babej <mbabej@xxxxxxxxxx> 2009-12-04 11:31:46 EDT --- (In reply to comment #3) > * %define -> %global > - Now Fedora prefers to use %global over %define. Fixed. > > * License > - test/haml/spec/README.md is under WTFPL so the license tag > should be "MIT and WTFPL". Fixed. > > * Requires > - Please add the needed rubygem related dependency. > Note that I don't know if this dependency is optional or not. /usr/bin/html2haml requires it, so i think it's not optional. Added. > Also please check other dependency (if any). I checked all files for requires, and other possible external tools: 1. extra/haml-mode.el and extra/sass-mode.el are emacs highlighting files; not sure what to do with these. 2. extra/update_watch.rb requires sinatra and json, but this file seems only useful to haml developers, i think, so i'd like to remove it. 3. lib/haml/filters.rb: this depends on rubygem-RedCloth for Textile filter, but Markdown and Maruku filters doesn't have packaged dependencies in fedora (afaict). The code in filters.rb can handle it though, and these filters are not a requirement to use haml. What do you suggest ? 4. Rakefile has many dependencies (tlsmail, yard, rcov/rcovtask, ruby-prof, "git" command...) and i'm not sure how useful it is. The only thing i'd like to keep is the 'test' task. 5. test/benchmark has more requires (erubis, markaby, rbench - i didn't find this packaged) 7. test/haml/spec/lua_haml_spec.lua - requires lua 8. test/haml/spec/ruby_haml_test.rb - requires json; doesn't work currently, but i have a patch to make this work 6. test/sass/plugin_test.rb has require merb, but it will skip the test if no merb is found. > > * %check > - Whether adding executable permission to a script or not should > be determined (for this case) by checking if the script has > shebang or not, and should not be determined by hardcoded file list. Not all files from *_test.rb have shebang (Though i could create a patch for this and ask upstream to integrate it) > I think > - fixing Rakefile and execute "rake test" I prefer this one (where by fixing i mean leave only "test" task) > > * Macros > - As %geminstdir is already defined, use the macro in %files. Fixed. > > * %changelog style Fixed. > By the way it is appreciated if you post the full URL of the new > spec/srpm. Sure. SRPM: http://mogurakun.web.runbox.net/rubygem-haml-2.2.15-2.fc12.src.rpm Spec: http://mogurakun.web.runbox.net/rubygem-haml.spec -- 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