Bug 224228 - Review Request: kgtk - Allows Gtk and Qt applications to use KDE's file dialogs
Review Request: kgtk - Allows Gtk and Qt applications to use KDE's file dialogs
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Karol Trzcionka
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-24 13:29 EST by Francois Aucamp
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-26 03:03:25 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Francois Aucamp 2007-01-24 13:29:57 EST
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 15:52:49 EST
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 17:38:48 EST
- 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 03:06:05 EST
(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 15:20:14 EST
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 01:53:20 EST
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 03:03:25 EST
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.