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 <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[email protected]
         AssignedTo|[email protected]    |[email protected]
               Flag|                            |fedora-review+

--- Comment #1 from Mohammed Morsi <[email protected]> 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.
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to