[Bug 543549] Review Request: rubygem-haml - XHTML/XML templating engine

[bugzilla at redhat.com]

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