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@xxxxxxxxxx> 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@xxxxxxxxxx> - 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review