[Bug 670930] Review Request: rubygem-regin - Ruby Regexp Introspection

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

Mohammed Morsi <mmorsi at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |mmorsi at redhat.com
         AssignedTo|nobody at fedoraproject.org    |mmorsi at redhat.com
               Flag|                            |fedora-review+

--- Comment #1 from Mohammed Morsi <mmorsi at redhat.com> 2011-01-23 16:35:14 EST ---
Will take this one.

This package looks good, rpmlint looks good, passes the review guidelines.

One minor nit, "rm -rf %{buildroot}" is no longer needed and can be removed
from the %install and %clean sections.

RE the tests, please consider the following Guideline

"SHOULD: The reviewer should test that the package functions as described. A
package should not segfault instead of running, for example."

from
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines#Things_To_Check_On_Review

Right now, rpms for gems which do not invoke the test suite are not being
verified as functional at build time. Note this is independent of whether or
not the test / spec suite is included in the final rpm or in a sub docs package
(or not included at all).

Please consider adding it, you can with the following patch to your spec

$ diff rpmbuild/SPECS/rubygem-regin.spec.orig rpmbuild/SPECS/rubygem-regin.spec
14a15,24
> # The regin gem doesn't ship with the upstream Rakefile
> Source1: http://github.com/josh/regin/raw/v%{version}/Rakefile
> 
> # You may check regin tests out like so
> # git clone http://github.com/josh/regin.git
> # cd josh
> # git checkout v0.3.7
> # tar czvf regin-tests.tgz spec/
> Source2: regin-tests.tgz
> 
36a47,57
> %setup -q -c -T
> 
> mkdir -p ./%{gemdir}
> gem install --local --install-dir ./%{gemdir} \
>             --force --rdoc %{SOURCE0}
> 
> # move the Rakefile in place
> cp %{SOURCE1} .%{geminstdir}
> 
> # move the tests into place
> tar xzvf %{SOURCE2} -C .%{geminstdir}
41d61
< rm -rf %{buildroot}
43,44c63
< gem install --local --install-dir %{buildroot}%{gemdir} \
<             --force --rdoc %{SOURCE0}
---
> cp -a .%{gemdir}/* %{buildroot}%{gemdir}
47d65
< rm -rf %{buildroot}
49a68,69
> pushd .%{geminstdir}
> rake test
62a83,84
> %doc %{geminstdir}/spec
> %doc %{geminstdir}/Rakefile



In any case, since I've verified the test suite works for this package version,
everything looks to be in order


APPROVED

-- 
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.