[Bug 190213] Review Request: gq - Graphical LDAP directory browser and editor

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: gq - Graphical LDAP directory browser and editor


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190213





------- Additional Comments From fedora at christoph-wickert.de  2006-11-19 19:00 EST -------
Thanks,

you are pretty fast with your update. You will have to update your package if
there will be a language pacak for 1.2.2. Anyway, here we go:

REVIEW for 
c1b303242245dd6fc2c04bb1a9d020b6  gq-1.2.2-1.fc6.src.rpm

FAIL - rpmlint isn't happy with your srpm:
$ rpmlint gq-1.2.2-1.fc6.src.rpm 
W: gq patch-not-applied Patch01: gq-1.2.1-desktop.patch

This is because the patch is defined as Patch01:, but it's called with %patch1.
You should change it to Patch0: anyway, since it's the first patch. Same for
Source: and Source01:, should be Source0: and Source1: instead.

rpmlint on the binaries is clean.

OK - package meets naming guidelines
OK - specfile named correctly
OK - package meets packaging guidelines
OK - license open-source compatible (GPL)
OK - license included in source
OK - license field in specfile machtes actual license
OK - license included in %doc
OK - specfile in American English
OK - specfile is legible
OK - source matches upstream
OK - package builds for Core 6 on i386
OK - BuildRequires are correct, not exeptions, no duplicates
OK - locales are handled correctly with %find_lang.sh
OK - package is not relocatable
OK - package owns all directories that it creates
OK - no duplicate files in the %files listing
OK - permissions are set properly, correct %defattr:
OK - %clean section present
OK - macro usage consistent (using macro style)
OK - code, not content
OK - no large docs
OK - docs don't affect the program's runtime
OK - no headers static libs or pc files
OK - no need for a devel package
OK - no libtool archives
OK - package correctly uses desktop-file-install
OK - package doesn't own dirs already owned by other packages
OK - package works as described
OK - package uses scriptlets from the wiki
FAIL - package doesn't build in mock, an error occurs while building the locales:

> Making all in po
> make[2]: Entering directory `/builddir/build/BUILD/gq-1.2.2/po'
> file=`echo cs | sed 's,.*/,,'`.gmo \
> 	  && rm -f $file &&  -o $file cs.po
> /bin/sh: line 1: -o: command not found
> make[2]: *** [cs.gmo] Error 127
> make[2]: Leaving directory `/builddir/build/BUILD/gq-1.2.2/po'
> make[1]: *** [all-recursive] Error 1
> make[1]: Leaving directory `/builddir/build/BUILD/gq-1.2.2'
> make: *** [all] Error 2
> error: Bad exit status from /var/tmp/rpm-tmp.23553 (%build)

the 4th line should be

> 	  && rm -f $file && /usr/bin/msgfmt -o $file cs.po
 
You need to BuildRequire gettext to provide msgfmt.

FIX - Remove the line about desktop-file-utils from changelog entry of 1.2.1-3.
The line is wrong, desktop-file-utils is still included in your spec and this is
correct. Simply drop that line.

FIX - Although the package builds in mock you should add a buildrequirement on
libgcrypt-devel (configure checks for /usr/bin/libgcrypt-config)

FIX? - You should also consider libgpg-error-devel, it's used if available too.
I have to admit don't know if it adds any value to the package, but IMO it can't
hurt.

MINOR - After you have removed useless macros you can also replace "%{__cp} -p"
with a simple "cp" (when copying LINGUAS)

MINOR - IMHO %post and %postun should not be after the %files section

MINOR - You can make %post and %post un a little smarter

> %post -p update-mime-database %{_datadir}/mime &> /dev/null || :
>
> %postun -p update-mime-database %{_datadir}/mime &> /dev/null || :

This has the advantage the rpm will automagically care for the requirement on
update-mime-database/shared-mime-info

Please fix the blockers (FAIL and FIX), so that I can approve the package.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.