[Bug 710902] Review Request: octave-struct - Structure handling for Octave

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #3 from Thomas Sailer <t.sailer@xxxxxxxxxxxxxx> 2011-06-13 13:45:33 EDT ---
(In reply to comment #2)

Thank you for taking the review!

> [!] : MUST - Rpmlint output is silent.

Please compare this to Bug 693798, the review of octave-image.

>         rpmlint octave-struct-1.0.9-1.fc16.i686.rpm
>         octave-struct.i686: W: obsolete-not-provided octave-forge

As Orion Poplawski argued in the above referred bug, the obsoletes is
necessary.

>         octave-struct.i686: W: hidden-file-or-dir
> /usr/share/octave/packages/struct-1.0.9/packinfo/.autoload
>         octave-struct.i686: E: zero-length
> /usr/share/octave/packages/struct-1.0.9/packinfo/.autoload

This is the way the octave packaging system works; octave-image also contains
such an empty .autoload file

>         octave-struct.i686: W: dangerous-command-in-%preun rm

This warning comes from the rpm macros installed by octave, and should be fixed
there, IMO. That was already discussed in bug 693798.


> [!] : SHOULD - SourceX / PatchY prefixed with %{name}.
>         Patch0:         octave-struct-nostrip.patch
> (octave-struct-nostrip.patch)

Changed to:
Patch0:         %{name}-nostrip.patch

> Issues:
> [!] : SHOULD - Patches link to upstream bugs/comments/lists or are otherwise
> justified.

Ok, I will take the "otherwise justified" route.

Upstream made a deliberate choice to strip binaries upon installation.
This however runs counter to our packaging guidelines (namely that we
do not want useless debug packages). This patch just patches out the
stripping on installation. This is, therefore, a fedora specific
change, that needs not be reported to upstream (they will ignore it anyway
as it runs counter to their decision)

> * You can remove octave from the BuildRequires field since octave-devel already
> requires it (as it is usual for all *-devel to depend on the non-devel part)

Removed BR octave

> * The description part could be expanded a bit (I am aware that this is what
> shows the project page).

I tried to come up with a slightly longer and more informative text.

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


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]