[Bug 541491] Review Request: rubygem-ruby_parser - A ruby parser written in pure ruby

[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=541491





--- Comment #2 from Matthew Kent <mkent at magoazul.com>  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.