Bug 612023

There's already a review request for tinycdb, but Adrien still needs a sponsor:
https://bugzilla.redhat.com/show_bug.cgi?id=573448

(In reply to comment #1)
> There's already a review request for tinycdb, but Adrien still needs a sponsor:
> https://bugzilla.redhat.com/show_bug.cgi?id=573448    
It seems Adrien don't reply for your last comment for several months:) 

Actually, We need this package be reviewed soon, this is a blocker for packaging qt-mobility which is a core component in meego 1.1.

OK, I suggest to contact Adrien directly and ask him whether he's still interested in maintaining this package. If not, we could close it as duplicate and review yours. Otherwise, a sponsor should be convinced to take the review in a timely manner. :)

*** Bug 573448 has been marked as a duplicate of this bug. ***

Your package looks almost fine to me. Just a couple of minor remarks:

- Add file debian/copyright as %doc. Even if "public domain" isn't a real license, the corresponding copyright information should be added to the binary package.

- The tarball contains a pkgconfig file in folder debian/. It's probably a good idea to add it to the package. It must be slightly adapted, though.

- I suggest to replace the second sentence of the devel %description with:
This package contains library and header files for developing applications that use %{name}.

(In reply to comment #5)
> Your package looks almost fine to me. Just a couple of minor remarks:
> 
> - Add file debian/copyright as %doc. Even if "public domain" isn't a real
> license, the corresponding copyright information should be added to the binary
> package.
> 
> - The tarball contains a pkgconfig file in folder debian/. It's probably a good
> idea to add it to the package. It must be slightly adapted, though.
> 
> - I suggest to replace the second sentence of the devel %description with:
> This package contains library and header files for developing applications that
> use %{name}.    

All fixed except license file which is not needed for this package.

From licensing guideline:

If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. If the source package does not include the text of the license(s), the packager should contact upstream and encourage them to correct this mistake. 

SRPM: http://dl.dropbox.com/u/1338197/1/tinycdb-0.77-2.fc13.src.rpm

(In reply to comment #6)
> If (and only if) the source package includes the text of the license(s) in its
> own file, then that file, containing the text of the license(s) for the package
> must be included in %doc. 

Yes, I know. But since "public domain" isn't a license, there's no license text that could be included. Thus, this part of the guidelines doesn't fit here. File debian/copyright is just a further %doc file like README or NEWS containing some information the user might be interested in. However, I leave it up to you to add it or not.

I'll do the formal review later today because I'm pretty busy at the moment.

The package looks OK now. Since %makeinstall usually should not be used, please add a short comment telling that it's valid here and works without unwanted side-effects.
I also recommend to be more specific in the %files section to avoid packaging of unwanted files in future versions, e.g.
%{_includedir}/*.h instead of %{_includedir}/*

$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
tinycdb.src: W: no-buildroot-tag
4 packages and 0 specfiles checked; 0 errors, 1 warnings.

If you don't plan to build this package for EPEL, the warning can be ignored.
Otherwise, add a buildroot tag and clean the buildroot in %install. Additionally, Requires: pkgconfig would be necessary.

---------------------------------
Key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: %makeinstall must not be used when make install DESTDIR=%{buildroot} works.
    - make install DESTDIR=%{buildroot} doesn't work
    - %makeinstall has no unwanted side-effects here, and simplifies the installation process

[+] MUST: The package must be licensed with a Fedora approved license.
[+] MUST: The License field in the package spec file must match the actual license.
[.] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum tinycdb_0.77.tar.gz*
    c00e5fb96c30356ac3b67b2ab5d5641b  tinycdb_0.77.tar.gz
    c00e5fb96c30356ac3b67b2ab5d5641b  tinycdb_0.77.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    koji scratch builds:
    F-12: http://koji.fedoraproject.org/koji/taskinfo?taskID=2316672
    F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2316685

[.] MUST: If the package does not successfully compile, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[.] MUST: The spec file MUST handle locales properly.
[+] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[+] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
    - static library removed

[+] MUST: Library files with a suffix (e.g. libfoo.so.1.1) must go in a -devel package.
[+] MUST: devel packages must require the base package using a fully versioned dependency
[+] MUST: Packages must NOT contain any .la libtool archives.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop file
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) ...
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described. 
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[+] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg. 
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin ...
[+] SHOULD: your package should contain man pages for binaries/scripts.

----------------
Package APPROVED
----------------

(In reply to comment #8)
> The package looks OK now. Since %makeinstall usually should not be used, please
> add a short comment telling that it's valid here and works without unwanted
> side-effects.
> I also recommend to be more specific in the %files section to avoid packaging
> of unwanted files in future versions, e.g.
> %{_includedir}/*.h instead of %{_includedir}/*
> 
Actually, tinycdb only has one head file - cdb.h, will be fixed in cvs.

> $ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
> tinycdb.src: W: no-buildroot-tag
> 4 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
> If you don't plan to build this package for EPEL, the warning can be ignored.
> Otherwise, add a buildroot tag and clean the buildroot in %install.
> Additionally, Requires: pkgconfig would be necessary.
> 

Requires: pkgconfig will be added by rpmbuild automatically now, thanks for the review.

New Package CVS Request
=======================
Package Name: tinycdb
Short Description: Utility and library for manipulating constant databases
Owners: supercyper
Branches: F-13
InitialCC:

(In reply to comment #9)
> Requires: pkgconfig will be added by rpmbuild automatically now
Yes, but not in EPEL 5. The remark about pkgconfig was related to the previous comment about EPEL.

> thanks for the review.
You're welcome.

CVS done (by process-cvs-requests.py).