Bug 230071
Summary: | Review Request: libkexiv2 - A library to manipulate EXIF/IPTC information | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rex Dieter <rdieter> |
Component: | Package Review | Assignee: | manuel wolfshant <manuel.wolfshant> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | mgarski |
Target Milestone: | --- | Flags: | manuel.wolfshant:
fedora-review+
dennis: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-03-09 13:06:48 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 231473 |
Description
Rex Dieter
2007-02-26 14:25:43 UTC
The Description is a bit terse. May I suggest "A wrapper for Exiv2 library to manipulate picture metadata, used by kipi-plugins, digiKam and other kipi host applications." ? It's just what is in the README. (: Besides, only digikam uses it, for now, but I'd rather not mention client apps, since that could likely change. I know. But their web site / sf repository is a bit more informative. If not for the presence of "exif" in the description, I would have had absolutely no idea what's the purpose of the library, especially since "exiv" rang no bells. In my opinion, the presence of "picture metadata" and/or "digikam" would be better hints. My suggestion was based on info retrieved from upstream's web site. OK, I'm convinced, and will include it in the next pkg update (after someone comits to doing a pkg review). Well, just to push you a bit 1. There is a typo error in %clean: rm -rf $FPM_BUILD_ROOT 2. mock build fails with: checking for gawk... gawk checking whether make sets $(MAKE)... yes checking for kde-config... not found configure: error: The important program kde-config was not found! Please check whether you installed KDE correctly. error: Bad exit status from /var/tmp/rpm-tmp.91758 (%build) => kdelibs is missing as BR. Adding that, build fails again, this time with: checking for vsnprintf... yes checking for snprintf... yes checking for X... configure: error: Can't find X includes. Please check your installation and add the correct path s! error: Bad exit status from /var/tmp/rpm-tmp.6018 (%build) => kdelibs-devel is missing as BR So, please add kdelibs-devel as BR, correct the FPM into RPM and I'll try to do a review. It would the first one for a package which relies on pkgconfig, so please be kind to me :) crud, I coulda sworn I did a mock build prior to package submission... oh well. (: Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libkexiv2.spec SRPM URL: http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/libkexiv2-0.1.1-2.src.rpm %changelog * Tue Feb 27 2007 Rex Dieter <rdieter[AT]fedoraproject.org> 0.1.1-2 - fix %%clean - update %%description - BR: kdelibs-devel First of all, it would have been nice if the URL of spec from comment #7 would have been the modified one. Second, I think that the Description tag in -devel should match the one from the main package. You have modified only the main one... As a comment prior to the review: examining the build log and then the sources, I have noticed that %configure looks for doxygen and some doxygen related scripts seem to exist in the admin directory. Unfortunately my experience with doxygen is limited to installing it via yum. Just adding doxygen as BR makes configure pick it, but nothing seems to be changed afterwards: nothing is built in the admin directory andthe build log has some failures which _seem_ to look like missing QT requirements. Explicitely requiring qt-devel does not help either and unfortunately the machine where I have done all the tests just suffered yet another kernel crash. OTOH the admin folder does not seem to contain anything else but a bunch of scripts so I guess there is no need to struggle with it (could be a leftover from a more comprehensive package from which this tar has been separated). If I am wrong here, I would appreciate if someone would correct me. Good: - rpmlint checks do not return anything on the source and binary rpm. the -devel package gives: W: libkexiv2-devel no-documentation which I guess that can be ignored. The doc is in the main package. - package meets naming guidelines - package meets packaging guidelines - license (GPL) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream, is the latest available version, sha1sum 7218bb8b81955fc4ef376f302bda9b94bd2b68bd libkexiv2-0.1.1.tar.bz2 - package compiles on devel (x86_64 and i386) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all files/directories that it creates, does not take ownership of other files/directories - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - not a GUI so no need for .desktop file - devel package ok - no .la files (correctly removed in %install) - post/postun ldconfig ok - devel correctly requires pkgconfig - separation of libs between main and devel packages respects versioning (once again, I kindly ask someone more experienced to verify and correct me if I am wrong) SHOULD - successfully builds in mock for fc6 and devel, i386 and x86_64 warning: I cannot verify if the libs are actually working, nothing to test them with - no scriptlets except usage of ldconfig which is OK - pkgconfig(.pc) files are in -devel - devel requires the base package using a fully versioned dependency. What I would like to see - including of AUTHORS, ChangeLog and maybe even the RELEASE.rev files as %doc - the same Description in -devel as in main Both are minor and non blockers, they can be fixed before uploading to cvs. From my point of view the package is OK (minus the minor caveats I have mentioned ). However I kindly ask someone more experienced to have a second look, therefore I will leave this bug in REVIEW state for another day or two. I take that back, description in %devel is just fine. I was looking in a wrong place. Sorry :( No comments in one week, switching to APPROVED. New Package CVS Request ======================= Package Name: libkexiv2 Short Description: A library to manipulate EXIF/IPTC information Owners: rdieter.edu Branches: FC-5 FC-6 devel InitialCC: mgarski Branched Imported, and building, thanks. |