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 #4 from Josà Matos <jamatos@xxxxxxxx> 2011-06-13 14:07:13 EDT --- (In reply to comment #3) > (In reply to comment #2) > > Thank you for taking the review! You are welcome. > > [!] : 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. I am aware that is why I did not consider them to be an issue. On retrospective I should have placed a note saying exactly that. > > [!] : 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) My point here is that a single comment like: # Avoid package stripping to have a useful debug package over the patch is enough. You can rephrase it as you wish, but you get the idea. > > * 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. Usually it is nice to have a link for the new sources. It is not required but it is nice. :-) -- 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