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

bugzilla at redhat.com bugzilla at redhat.com
Wed Dec 9 18:14:03 UTC 2009


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 #6 from Michal Babej <mbabej at redhat.com>  2009-12-09 13:14:01 EDT ---
(In reply to comment #5)
> 
>   - It seems filters related dependency is optional, so I don't think
>     adding "R: rubygem(RedCloth)" is strictly needed.

OK.

> 
> > 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.
> 
>   - We usually don't add dependency for Rakefile based dependency
>     ( By the way I like to create -doc subpackage for rubygem based rpm
>       and I usually put Rakefile to -doc, not to main package )

The problem is, i can't even do "rake test" because loading of rakefile fails
on dependencies.

> 
>   - But with your current rpm only ruby script with shebang have executable
>     permission (and not all *_test.rb have shebang) anyway, so judging by
>     my method should be possible.

In my current rpm, only two of 10 test files miss shebang line, in %install i
add it to those two, and then set all 10 to +x with chmod.

>     ( By the way if scripts without shebang have executable permission, or
>       if scripts with shebang don't have exectable permission, rpmlint
>       warns about this ).

I know. That's why i do all this :)

> 
>     The reason I am talking about this is that I think hardcoding %test_files
>     list should be avoided unless impossible.  

It's not impossible, but i'd have to:
1. create a patch that adds shebang lines, so all _test.rb have it; and
2. call chmod +x, or specify executable bit with %attr

Do you think this would be OK ?

-- 
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 Fedora-package-review mailing list