Bug 193109 - Review Request: plotmm
Summary: Review Request: plotmm
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ralf Corsepius
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-25 10:59 UTC by Haïkel Guémar
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-07-30 21:54:58 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Haïkel Guémar 2006-05-25 10:59:55 UTC
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 04:02:37 UTC
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 14:40:45 UTC
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 15:07:58 UTC
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 04:11:08 UTC
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 04:13:58 UTC
------- Additional Comments From rc040203  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 04:14:44 UTC
------- Additional Comments From karlthered  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 04:15:29 UTC
------- Additional Comments From rc040203  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 05:04:55 UTC
Did someone agree to sponsor Haïkel?

Comment 9 Ralf Corsepius 2006-06-24 07:33:43 UTC
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 14:04:31 UTC
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 14:29:21 UTC
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 06:03:57 UTC
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 12:18:09 UTC
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 06:14:20 UTC
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 21:54:58 UTC
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.