Bug 670930
| Summary: | Review Request: rubygem-regin - Ruby Regexp Introspection | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Vít Ondruch <vondruch> |
| Component: | Package Review | Assignee: | Mo Morsi <mmorsi> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, mmorsi, notting |
| Target Milestone: | --- | Flags: | mmorsi:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-01-26 18:31:52 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 667997 | ||
|
Description
Vít Ondruch
2011-01-19 17:10:22 UTC
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
I have removed the "rm -rf" and added the specs as I proposed in Ruby-SIG. Please see updated versions if you like: Spec URL: http://people.redhat.com/vondruch/rubygem-regin.spec SRPM URL: http://people.redhat.com/vondruch/rubygem-regin-0.3.7-2.fc14.src.rpm Nevertheless, I am going to request the repository. Thank you for your review. New Package SCM Request ======================= Package Name: rubygem-regin Short Description: Ruby Regexp Introspection Owners: vondruch Branches: Git done (by process-git-requests). It looks like you pushed regin to rawhide. Thanks alot. Closing. |