Bug 193109 - Review Request: plotmm
Review Request: plotmm
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ralf Corsepius
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-25 06:59 EDT by Haïkel Guémar
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-30 17:54:58 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 Haïkel Guémar 2006-05-25 06:59:55 EDT
Spec URL: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SPECS/plotmm.spec
SRPM URL: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/plotmm-0.1.2-1.src.rpm
Description: GTKmm plot widget for scientific applications.
This is one of my first packages, I need a sponsor
Comment 1 Ralf Corsepius 2006-05-26 00:02:37 EDT
NEEDSWORK:

Blockers:

- The package ships example applications in %{_bindir}
/usr/bin/simple
/usr/bin/curve

To me this seems questionable twice:
1. These application names are too general and likely to conflict with other
packages.
2. Installing example applications to %{_bindir} isn't necessarily a wise decision.

Wrt. 2, I'd propose to move these examples out of the base package into an
*-examples package or to make them part of the *-devel package.

Wrt. 1., I'd propose to move these applications out of %{_bindir} and to install
them into a different location (e.g. %{_libdir}/plotmm/examples), or to rename
them into something less likely to conflict (e.g. plotmm-{simple|curve}).


Minor:
- The sources contain devel docs (in doc/html/), but they are not being
packages. I'd propose to ship these docs as part of the *-devel package

- Rpmlint complaints:

# rpmlint plotmm-0.1.2-1.i386.rpm
W: plotmm summary-ended-with-dot GTKmm plot widget for scientific applications.
E: plotmm description-line-too-long It contains widgets which are primarily
useful for technical and scientifical purposes.
E: plotmm zero-length /usr/share/doc/plotmm-0.1.2/NEWS
E: plotmm zero-length /usr/share/doc/plotmm-0.1.2/INSTALL

# rpmlint plotmm-devel-0.1.2-1.i386.rpm
W: plotmm-devel summary-ended-with-dot Headers for developing programs that will
use plotmm.
E: plotmm-devel description-line-too-long This package contains the headers that
programmers will need to develop applications which will use plotmm.
W: plotmm-devel no-documentation

General issue: You are using an Alias-name instead of your real name in
%changelog. I am opposed to this practice.
Comment 2 Haïkel Guémar 2006-05-26 10:40:45 EDT
new SRPM: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/plotmm-0.1.2-2.src.rpm
* Blockers:
- samples applications are now in a separated package: plotmm-examples and moved
them to %{_libdir}/plotmm/examples, and changed their name into plotmm-*
- added the documentation in the devel package
* rpmlint complaints:
- the named issues were fixed
- rpmlint output about current packages
$ rpmlint plotmm-0.1.2-2.i386.rpm
W: plotmm one-line-command-in-%post /sbin/ldconfig
W: plotmm one-line-command-in-%postun /sbin/ldconfig
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-devel-0.1.2-2.i386.rpm
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-examples-0.1.2-2.i386.rpm
W: plotmm-examples no-documentation
* general issue
- This was an oversight, I have corrected it in plotmm and other packages.
Comment 3 Haïkel Guémar 2006-05-26 11:07:58 EDT
new SRPM: http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/plotmm-0.1.2-2.src.rpm
* Blockers:
- samples applications are now in a separated package: plotmm-examples and moved
them to %{_libdir}/plotmm/examples, and changed their name into plotmm-*
- added the documentation in the devel package
* rpmlint complaints:
- the named issues were fixed
- rpmlint output about current packages
$ rpmlint plotmm-0.1.2-2.i386.rpm
W: plotmm one-line-command-in-%post /sbin/ldconfig
W: plotmm one-line-command-in-%postun /sbin/ldconfig
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-devel-0.1.2-2.i386.rpm
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-examples-0.1.2-2.i386.rpm
W: plotmm-examples no-documentation
* general issue
- This was an oversight, I have corrected it in plotmm and other packages.
Comment 4 Chitlesh GOORAH 2006-06-14 00:11:08 EDT
Due to a bugzilla backup restored, Im copy/pasting what has been posted before:

> * rpmlint complaints:

------------
chitlesh(i386)[0]$rpmlint -i plotmm-0.1.2-2.i386.rpm
W: plotmm one-line-command-in-%post /sbin/ldconfig
You should use %post -p <command> instead of using:

%post
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command.

W: plotmm one-line-command-in-%postun /sbin/ldconfig
You should use %postun -p <command> instead of using:

%postun
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command.
------------

Use
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
to solve them
Comment 5 Chitlesh GOORAH 2006-06-14 00:13:58 EDT
------- Additional Comments From rc040203@freenet.de  2006-06-13 01:51 EST -------
(In reply to comment #4)
> (In reply to comment #3)
> ------------
> chitlesh(i386)[0]$rpmlint -i plotmm-0.1.2-2.i386.rpm
> W: plotmm one-line-command-in-%post /sbin/ldconfig
> You should use %post -p <command> instead of using:
Note the "should" - rpmlint is being stylish, here.

Sorry for not having completed the review earlier, some time, later today.
Comment 6 Chitlesh GOORAH 2006-06-14 00:14:44 EDT
------- Additional Comments From karlthered@gmail.com  2006-06-13 08:30 EST -------
I applied the changes -this will be corrected in other packages too-, here's the
new SRPM: /home/isshin/rpmbuild/RPMS/i386/plotmm-examples-0.1.2-3.i386.rpm
- rpmlint output:
$ rpmlint plotmm-0.1.2-2.i386.rpm
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-devel-0.1.2-2.i386.rpm
$ rpmlint /home/isshin/rpmbuild/RPMS/i386/plotmm-examples-0.1.2-2.i386.rpm
W: plotmm-examples no-documentation
Comment 7 Chitlesh GOORAH 2006-06-14 00:15:29 EDT
------- Additional Comments From rc040203@freenet.de  2006-06-13 10:10 EST -------
I am having a couple of issues with the spec:

- /usr/share/doc/plotmm-devel-0.1.2/doc/html/
That's at least one directory too much for my taste.

Using
%doc doc/html instead of %doc
will install the html docs to
/usr/share/doc/plotmm-devel-0.1.2/html

- Empty %doc for the *-examples
%doc
Remove this line.

- You went a bit too far, as far as renaming the examples is concerned.
My remark in comment #1 was addressing conflicts between real applications and
example code. Therefore I had proposed to either "rename" or "move" the example
applications. Now, you've done both.
Comment 8 Jason Tibbitts 2006-06-24 01:04:55 EDT
Did someone agree to sponsor Haïkel?
Comment 9 Ralf Corsepius 2006-06-24 03:33:43 EDT
I am already formally reviewing this package and could sponsor him, but he has
not yet responded to my comment in comment #7.
Comment 10 Haïkel Guémar 2006-06-29 10:04:31 EDT
Sorry for the delay, I had been quite busy.
Here's the new SRPM with the suggested fixes.
http://darkenphoenix.free.fr/RPMS/RPMS/Extras/SRPMS/plotmm-0.1.2-4.src.rpm
Anyway, I have also other packages that need to be reviewed:
bugs 193108 (libsexymm C++ bindings to libsexy ), 193109 (python-sexy python
bindings to libsexy), 193103 (listen yet another music player)
Comment 11 Ralf Corsepius 2006-07-09 10:29:21 EDT
I had preferred, if you installed the examples to somewhere outside of
%{_bindir} instead of renaming them, because this would have helped to keep
%{_bindir} free of "more or less useless examples", but ... this basically is a
matter of personal preference.

APPROVED.
Comment 12 Ralf Corsepius 2006-07-17 02:03:57 EDT
Haïkel, are you still interested in pushing this package into FE?

Then I'd recommend you to proceed with processing this request as described on
http://fedoraproject.org/wiki/Extras/Contributors

Shouldn't I hear from you within 14 days from now, I'll close this PR as FAILED.
Comment 13 Haïkel Guémar 2006-07-18 08:18:09 EDT
I'm still interested to maintain this package.
I have signed the CLA, and added myself to cvsextras and fedorabugs groups (step
4.a/b of the howto)
Comment 14 Ralf Corsepius 2006-07-24 02:14:20 EDT
Haïkel, you have been sponsored, now you are supposed to import this package
into CVS and build it.
Comment 15 Haïkel Guémar 2006-07-30 17:54:58 EDT
The build has succeeded for both FC5 and developpement tree. 
Thanks for everything guys !

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