Bug 188668 - Review Request: zenity
Summary: Review Request: zenity
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Cantrell
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FC-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-04-12 01:16 UTC by Matthias Clasen
Modified: 2013-01-10 01:22 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-04-19 17:45:03 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Matthias Clasen 2006-04-12 01:16:51 UTC
Spec URL: http://people.redhat.com/mclasen/review/zenity.spec
SRPM URL: http://people.redhat.com/mclasen/review/zenity-2.14.1-2.src.rpm
Description: This is part of the effort to split the current hard to maintain
multitarball gnome-utils packages into separate packages.

Comment 1 Jesse Keating 2006-04-17 17:45:10 UTC
Good:
- Package builds w/ mock on 32 and 64bit
- rpmlint is silent.

Bad:
- Missing autoconf as a BuildRequires

Question:
Would the postun and post cause scrollkeeper-update to run 2x each time this
package is upgraded?  Once for the install and once for the removal?  If so,
shouldn't this be logic wrapped so that the postun call only happens when it is
flat out removed?


Comment 2 Matthias Clasen 2006-04-17 19:01:19 UTC
new spec: http://people.redhat.com/mclasen/review/zenity.spec
new srpm: http://people.redhat.com/mclasen/review/zenity-2.14.1-3.src.rpm

changes: add BuildRequires: autoconf

We certainly could do better with the scrollkeeper-update thing, but that is
not the current practise. In general, having a working transaction-hook would
be nice on many counts. 

Comment 3 Yanko Kaneti 2006-04-17 19:53:23 UTC
There are packaging guidelines you know.
Including scriptlet snippets:
http://fedoraproject.org/wiki/ScriptletSnippets
and specifically one for scrollkeeper:
http://fedoraproject.org/wiki/ScriptletSnippets#head-3c9f517f0cd4aaabb369a8805226d85dc2f02793

Comment 4 Jesse Keating 2006-04-17 21:33:43 UTC
Yes, it appears that this package needs to follow the guidelines for scrollkeeper.

Specifcally the Requires(pre) and Requires(post).

Comment 5 Brian Pepple 2006-04-17 21:47:33 UTC
You've also got some unnecessary BuildRequirements.  glib2-devel, gtk2-devel,
libglade2-devel can all be dropped since libgnomecanvas-devel will pull these
all in.

Comment 6 Matthias Clasen 2006-04-18 04:43:21 UTC
I know there are packaging guidelines, but I don't know how I am supposed to
consider the ScriptletSnipplets page to be part of them. It isn't even linked from 
the Packaging/Guidelines page, so how should I even find it ?!

I disagree wrt to the BuildRequires. A direct dependency should still be listed,
even if it would also be pulled in indirectly. 

Comment 7 Brian Pepple 2006-04-18 12:42:16 UTC
(In reply to comment #6)
> I know there are packaging guidelines, but I don't know how I am supposed to
> consider the ScriptletSnipplets page to be part of them. It isn't even linked
from 
> the Packaging/Guidelines page, so how should I even find it ?!

It' s the first sub-heading under Packaging Guidelines on the Fedora Extras page.
http://fedoraproject.org/wiki/Extras

Comment 8 Matthias Clasen 2006-04-18 12:46:07 UTC
Now go to
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28guidelines%29%7C%28packaging%29
and tell me if you see a link there.

Anyway 
http://people.redhat.com/mclasen/review/zenity.spec
http://people.redhat.com/mclasen/review/zenity-2.14.1-4.src.rpm

has Requires(post) and Requires(postun) now (not pre and post, as
comment #4 wrongly suggested)


Comment 9 Yanko Kaneti 2006-04-18 13:23:04 UTC
What are your objections against using common scriptlets?
The fact that they are not linked directly I would consider a bug.

Comment 10 Matthias Clasen 2006-04-18 13:40:52 UTC
I have no objections to helpful hints. 
I do have objections to blindly following orders.
What in particular do you think is amiss in the scriplets in my latest spec ?


Comment 11 Yanko Kaneti 2006-04-18 14:11:10 UTC
My first comment was ment as a hint. I realize now it could be considered snide.
I apologize.

Your scripts are just different than the ones on the common scriptlets page. I
just don't consider the realm of sciptlets in packaging for a widely used common
distribution base an appropriate place for creative differences. Every such
difference makes cornercases for everyone and everything that tries to deal with
these specs. Be it human or automatic.

If you have objections against the common scriptlets why not discuss them rather
than creating yet another variety.

But thats just me. Anal retentive if you wish..

Comment 12 Jesse Keating 2006-04-18 14:37:59 UTC
Changes look good, rpmlint is silent, package builds w/ mock.  Scriptlets work
even if they aren't EXACTLY like the suggested ones, thats passible.


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