[Bug 742189] Review Request: rubygem-webmock - Library for stubbing HTTP requests in Ruby
bugzilla at redhat.com
bugzilla at redhat.com
Mon Oct 3 11:20:57 UTC 2011
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=742189
--- Comment #2 from Mo Morsi <mmorsi at redhat.com> 2011-10-03 07:20:56 EDT ---
New Spec / SPRM:
Spec URL: http://mo.morsi.org/files/rpms/rubygem-webmock.spec
SRPM URL:
http://mo.morsi.org/files/rpms/rubygem-webmock-1.7.6-2.fc15.src.rpm
> rubygem-webmock.noarch: W: doc-file-dependency
> /usr/lib/ruby/gems/1.8/gems/webmock-1.7.6/Rakefile /usr/bin/env
>
> From rpmlint source ... '''An included file marked as %doc creates a possible
> additional dependency in the package. Usually, this is not wanted and may be
> caused by eg. example scripts with executable bits set included in the
> package's documentation.'''
Fixed, unmarked Rakefile as doc. Also changed /usr/bin/env rake to
/usr/bin/rake
>
> rubygem-webmock-doc.noarch: W: unexpanded-macro
> /usr/lib/ruby/gems/1.8/doc/webmock-1.7.6/ri/WebMock/RequestSignature/eql%3f-i.yaml
> %3f
> <snip>
>
> From rpmlint source ... '''This package contains a file whose path contains
> something that looks like an unexpanded macro; this is often the sign of a
> misspelling. Please check your specfile.'''
>
These can be ignored, they occur in any rubygem that ships ri documentation
> > [ FAIL ] MUST: The License field in the package spec file must match the
> > actual license
>
> The upstream LICENSE file seems to indicate MIT, does this need to be updated?
>
Fixed
> Note, there are too many files listed as %doc. For example, the Rakefile
> probably shouldn't be a %doc. Maybe the same with other source code?
>
> %exclude %{geminstdir}/Rakefile
>
Unmarked Rakefile as doc, the others are appropriately marked as doc (include
tests and such)
> > [ FAIL ] MUST: Permissions on files must be set properly. Executables should
> > be set with executable permissions, for example. Every %files section
> > must include a %defattr(...) line.
This is no longer needed
http://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions
> > [ FAIL ] MUST: At the beginning of %install, each package MUST run rm -rf
> > %{buildroot} (or $RPM_BUILD_ROOT).
>
> Fixed, see http://fpaste.org/GNSM/
>
This is no longer needed / should not be present
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> > [ WARN ] - The Ruby library files in a pure Ruby package must be placed into Config::CONFIG["sitelibdir"] . The specfile must get that path using %{!?ruby_sitelib: %global ruby_sitelib %(ruby -rrbconfig -e 'puts Config::CONFIG["sitelibdir"] ')}
>
> The specfile is using a different method for locating /usr/lib/ruby/*. Should
> it be using %{ruby_sitelib} instead? Or does this not apply since this is
> providing a rubygem?
>
Yes according to the Fedora gem packaging guidelines, we define gemdir and
geminstdir correctly
http://fedoraproject.org/wiki/Packaging:Ruby#Ruby_Gems
> > [ WARN ] - The %prep and %build sections of the specfile should be empty.
>
> %build is empty, %prep is not ... I've adjusted per the ruby guidelines
> slightly. However this is a *should* requirement, not a *must*.
>
I would prefer to leave it as it is. The reason being if we ever have to patch
the gem, the gem install needs to occur in the %prep section before we can run
the %patch commands there as well.
> > [ FAIL ] - The install should be performed with the command 'gem install --local --install-dir %{buildroot}%{gemdir} --force %{SOURCE0}'
>
> This command is currently used in the %prep. I've adjusted slighty to
> accommodate the *should* requirement. Feel free to use if desired.
>
> http://fpaste.org/GNSM/
>
Again would prefer to leave as is unless this is a major blocker.
Thank you greatly for the review!
--
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.
More information about the package-review
mailing list