Bug 201842 - Review Request: plotutils - GNU plotutils graphics libs and utils
Review Request: plotutils - GNU plotutils graphics libs and utils
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul F. Johnson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 196510
  Show dependency treegraph
 
Reported: 2006-08-09 08:27 EDT by Denis Leroy
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-18 07:40:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Denis Leroy 2006-08-09 08:27:32 EDT
Spec URL: http://www.poolshark.org/src/plotutils.spec
SRPM URL: http://www.poolshark.org/src/plotutils-2.5-1.src.rpm

Description: The GNU plotutils package contains software for both programmers and
technical users. Its centerpiece is libplot, a powerful C/C++ function
library for exporting 2-D vector graphics in many file formats, both
vector and raster. It can also do vector graphics animations. Besides
libplot, the package contains command-line programs for plotting
scientific data. Many of them use libplot to export graphics.

Notes to reviewer:
- the cp's in the %prep section replace a symlink used in the source tree to share the same source file across multiple directories. This was breaking the debuginfo package.
- examples moved to /usr/share/doc where they belong.
Comment 1 Paul F. Johnson 2006-08-09 18:28:56 EDT
Looking at the spec file

You need Requires (post) : ldconfig unless you change post to be -p /sbin/ldconfig

mkdir docs-to-include

Where exactly does that get made? Why not just create a directory in the
buildroot and shift from there rather than interfere with the dearchived source?
This is not a complaint, just a suggestion. Other than patches, doing things in
the BUILD I've always considered a bad idea (TM)
Comment 2 Paul F. Johnson 2006-08-10 06:41:20 EDT
rpmlint on the src package is giving the warning that you've mixed spaces with tabs

builds cleaning in mock
installs cleanly using yun localinstall
rpm -qa --requires is not showing anything untoward

I'll need to do the full review when I'm sitting infront of the machine.

Can you address the mixed spaces and tabs and just upload a new spec file?
Comment 3 Denis Leroy 2006-08-10 09:30:29 EDT
SPEC: http://www.poolshark.org/src/plotutils.spec
SRPM: http://www.poolshark.org/src/plotutils-2.5-2.src.rpm

> warning that you've mixed spaces with tabs

Somehow I always get that even when I use tabs everywhere! :-) Fixed.

> You need Requires (post) : ldconfig

Fixed.

> mkdir docs-to-include

I think it's a pretty harmless trick to add directories straight into
/usr/share/doc/%{name}. I use that already in the gtkmm packages. Now I'm not
sure it would work the same way if you created it under BUILDROOT, because %doc
works differently if you give it a relative or absolute path, and the relative
path is always from the source dir... I think you'd have to explicitely move
things from %{_datadir} into %{_docdir}/%{name}/ after installation.
Comment 4 Denis Leroy 2006-08-16 07:21:18 EDT
Ping :-) (we're so close!)
Comment 5 Paul F. Johnson 2006-08-16 18:23:23 EDT
Sorry - not been that well recently, I'm usually out of it by about 9.30pm
(British Summer Time). It's 2nd on my list for tomorrow's reviews. Who knows, it
may even be good to go (TM) by this time tomorrow!
Comment 6 Paul F. Johnson 2006-08-17 18:41:03 EDT
Good

Software works (tested against Ortep)
Consistent use of macros
Includes documentation
No static libs/.la files
No dupes from the rpms
spec file in american english
rpmlint clean
mock builds fine
Includes devel file, no .pc file, so no pkgconfig required for devel
Correct use of scriptlets
Dist tag fine
Doesn't leave a mess when deinstalling

I'm happy with this.

APPROVED

Please remember to close the bug and change the tab to NEXT-RELEASE.
Comment 7 Denis Leroy 2006-08-18 07:40:07 EDT
Paul, thanks for your review!

Note You need to log in before you can comment on or make changes to this bug.