Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 224228 - Review Request: kgtk - Allows Gtk and Qt applications to use KDE's file dialogs
Summary: Review Request: kgtk - Allows Gtk and Qt applications to use KDE's file dialogs
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Karol Trzcionka
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-24 18:29 UTC by Francois Aucamp
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-26 08:03:25 UTC


Attachments (Terms of Use)

Description Francois Aucamp 2007-01-24 18:29:57 UTC
Spec URL: http://www.snoekie.com/rpm/kgtk.spec
SRPM URL: http://www.snoekie.com/rpm/kgtk-0.8-1.src.rpm
Description:
This is an LD_PRELOAD library that allows Gtk and Qt applications (such as
Firefox, Evolution, Eclipse, etc) to use KDE's file dialogs.

When a Gtk/Qt app is executed, it starts the kdialogd app (if not already
started), and communicates with this via a UNIX domain socket. There will only
ever be one instance of kdialogd, and all apps communicate with the same
instance - and it terminates itself 30 seconds after the last Gtk/Qt app has
disconnected.


This package builds in mock (fc6/i386) and has been built and tested on fc6/i386 and fc5/x86_64. rpmlint is silent.

Comment 1 Karol Trzcionka 2007-01-24 20:52:49 UTC
I will review it later. I can see that the *.so is not in devel subpackage. You
must fix it. 

Comment 2 Karol Trzcionka 2007-01-24 22:38:48 UTC
- MUST: rpmlint is silent - OK
- MUST: The package named according to the Package Naming Guidelines. - OK
- MUST: The spec file name in the format %{name}.spec - OK
- MUST: The package meets the Packaging Guidelines. - OK
- MUST: licensed with an open-source compatible license (GPL) - OK
- MUST: The License field in the package spec file matches the actual license. - OK
- MUST: Text of license is included in %doc - OK
- MUST: The spec file is written in American English. - OK
- MUST: The spec file for the package is legible. - OK
- MUST: The sources used to build the package matches the upstream source, as
provided in the spec URL. (2cde8a09508773cf2f9028912be4fbbe)
- MUST: successfully compile and build on i386/fc6 (in mock) - OK
- MUST: Not ExcludeArch - OK
- MUST: All build dependencies are listed in BuildRequires (mock-build correct) - OK
- MUST: There are not any locales - OK
- MUST: ldconfig non required - OK
- MUST: the package not designed to be relocatable - OK
- MUST: A package owns all directories that it creates - OK
- MUST: There are not any duplicates in %files. - OK
- MUST: Permissions on files set properly - OK
- MUST: %clean section with rm -rf %{buildroot} - OK
- MUST: macros use consistently - OK
- MUST: The package contains code, or permissable content. - OK
- MUST: -doc subpackage not required - OK
- MUST: Files in %doc not affects the runtime of the application - OK
- MUST: Not any static libraries ot headers - OK
- MUST: Non *.pc files - OK
- MUST: library files end in .so (without suffix) are not in -devel - TO BE FIXED
- MUST: not any subpackages (but requires)
- MUST: not contain any .la libtool archives - OK
- MUST: Packages is not containing GUI applications - OK
- MUST: Packages does not own files or directories already owned by other
packages. - OK

SHOULD Items:
- SHOULD: text of license included in package
- SHOULD: any translations
- SHOULD: Package tested in mock
- SHOULD: The package should compile and build into binary rpms on all supported
architectures. (At now I cannot check it)
- SHOULD: Package runs correctly (not any errors)
- SHOULD: scriptlets are not used
- SHOULD: Not any subpackages (without devel which must be created)
- SHOULD: Not any *.pc files



Comment 3 Francois Aucamp 2007-01-25 08:06:05 UTC
(In reply to comment #1)
> I will review it later. I can see that the *.so is not in devel subpackage. You
> must fix it. 

No. The .so files are not development libraries; they are merely LD_PRELOAD
modules that get loaded when another program is executed (if needed). If you
look at the wrapper scripts /usr/bin/kgtk-wrapper and /usr/bin/kqt-wrapper
you'll see what I mean.

I have, however, patched an error in the libkgtk module's Makefile.in that
caused it to create (useless) library version information.

New build:
Spec URL: http://www.snoekie.com/rpm/kgtk.spec
SRPM URL: http://www.snoekie.com/rpm/kgtk-0.8-2.src.rpm

Changes:
- Created patch to prevent useless libkgtk.so.* symlinks during %%build
- Added package name/version prefix to patch filenames


Comment 4 Karol Trzcionka 2007-01-25 20:20:14 UTC
My mistake, I did not read files :)
With new patch the files are more clear.

   This package (kgtk) is APPROVED by me.

Comment 5 Francois Aucamp 2007-01-26 06:53:20 UTC
Thanks for the review! Will import, build and close the the bug as soon as it's
successful.

Comment 6 Francois Aucamp 2007-01-26 08:03:25 UTC
Successfully built for devel, branches requested. Closing review.


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