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@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |mmorsi@xxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |mmorsi@xxxxxxxxxx Flag| |fedora-review+ --- Comment #1 from Mohammed Morsi <mmorsi@xxxxxxxxxx> 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 package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review