[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 #6 from Michal Fojtik <mfojtik at redhat.com> 2010-10-14 06:49:25 EDT ---
(In reply to comment #5)
> 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

Fixed. Thanks for info btw, I didn't know that there is an autodetection.

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

Agree. Fixed.

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

Fixed. Thanks for contributing this. I fully agree that those tests should be
preserved in -doc package for futher testing.

> ----------------------------------------------------------
>   ! By the way, $ rake spec needs "BuildRequires: rubygem(rake)".

Fixed.

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

Fixed.

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

Fixed.

======================= 0.1.31-3 ====================

* Thu Oct 14 2010 Michal Fojtik <mfojtik at redhat.com> - 0.1.31-3
- Preserved failing test files (thx. to mtasaka)
- Fixed macros usage
- Replaced path with macro
- Removed libcurl from requires

Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2534502

Spec URL: http://mifo.sk/RPMS/rubygem-typhoeus.spec
SRPM URL: http://mifo.sk/RPMS/rubygem-typhoeus-0.1.31-3.fc13.src.rpm

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