[Bug 641295] Review Request: rubygem-typhoeus - A library for interacting with web services at blinding speed

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

--- Comment #5 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 2010-10-13 16:12:47 EDT ---
For -2:

* Requires
  - "Requires: libcurl" is redundant and should be removed, because
    rpmbuild detects and adds this dependency automatically based on
    the dependency of the libraries:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Requires

* Applying patch
  - It is better that patches are applied in %prep using %patchX
    macro.

? Tests needing Rails or so to be running
  - Well, I am not sure if it is preferable to remove tests
    which needs Rails or so to be running. 
    - When people makes Rails or so running, he/she may want
      to check typhoeus using these test scripts.
    Maybe it is better that
    - we don't remove such tests
    - and ignore the failure case for now
    - Or:
----------------------------------------------------------
%check
pushd .%{geminstdir}

NEEDSKIP=""
for needskip in \
  spec/typhoeus/request_spec.rb \
  spec/typhoeus/hydra_spec.rb \
  spec/typhoeus/remote_spec.rb \
  spec/typhoeus/multi_spec.rb \
  spec/typhoeus/easy_spec.rb \
  spec/typhoeus/remote_proxy_object_spec.rb
do
  NEEDSKIP="$NEEDSKIP $needskip"
done

for needskip in $NEEDSKIP
do
  mv $needskip ${needskip}.save
done

rake spec --trace

for needskip in $NEEDSKIP
do
  mv ${needskip}.save $needskip
done
----------------------------------------------------------
  ! By the way, $ rake spec needs "BuildRequires: rubygem(rake)".

* Consistent macro usage
  - Please use macros consistently. For example:
-----------------------------------------------------------
%files
%dir %{ruby_sitearch}/%{gemname}
%{ruby_sitearch}/typhoeus/*.so
-----------------------------------------------------------
    please use %{gemname} also in the third line.

* Miscs
-----------------------------------------------------------
    56  %install
    57  rm -rf %{buildroot}
    58  mkdir -p %{buildroot}%{ruby_sitearch}/%{gemname}
    59  mkdir -p %{buildroot}%{gemdir}
    60  mkdir -p %{buildroot}%{_prefix} 
-----------------------------------------------------------
  - The line 68 is unneeded.

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