[Bug 676308] Review Request: rubygem-net-scp - A pure Ruby implementation of the SCP client protocol

bugzilla at redhat.com bugzilla at redhat.com
Thu Mar 17 10:53:46 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=676308

--- Comment #3 from Vít Ondruch <vondruch at redhat.com> 2011-03-17 06:53:45 EDT ---
(In reply to comment #1)
> If you are still interested in importing this package into
> Fedora, I will have a look at this one. Instead I will appreciate
> it if you would review one of my review requests (e.g. bug 678678)

Yes, I am still interested in import. Thank you for your comments. Here is
updated version:

Spec URL: http://people.redhat.com/vondruch/rubygem-net-scp.spec
SRPM URL:
http://people.redhat.com/vondruch/rubygem-net-scp-1.0.4-2.fc15.src.rpm

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

> Some notes:
> 
> * Explicit version-specific Requires
>   - Please consider if you really have to write explicit
>     version-specific Requires like rubygems ">= 1.2".
>     * Usually when packages shipped on currently supported
>       Fedora branches all satisfy such version requirements,
>       we don't add such explicit version Requires:
>       https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

I removed the explicit versions. You are right.

> * Remove obsolete stuff
>   - rm -rf %{buildroot} at the top of %install is no longer needed
>     (if you intend to import this only on Fedora).

Done

> ! Directly installing gem archive into %buildroot
>   - I always recommend to install gem archive once under %_builddir
>     to make it sure that no additional files are generated or files
>     are not modified after installation is done.
>     * For example, some testsuites generate additional executables
>       or log files or modify files, which frequently confuses %files 
>       entry or leads to rpmbuild failure.
> 
>     And because of the same reason, it is recommended that %buildroot
>     is touched only in %install section (i.e. executing %check under
>     %buildroot is discouraged).

I agree with you if the gem has binary extension, but this is not the case.

Unfortunately your statement is double-edged sword. You want to ensure that the
test suite you are executing is testing the gem, you are going to install to
our users. However, during install phase, there are typically done some changes
in folder structure, some adjustments in path etc. If this is true, then
executing of test suite in build folders is worthless, because it does not
ensure anything. There might be bugs introduces by the install step.

On the other hand, you can done the restructuring also during build phase, but
then, what is the purpose of the install section? Just copy the already
prepared folder structure? There is no point.

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