[Bug 499579] Review Request: libxdg-basedir - Implementation of the XDG Base Directory Specifications

[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.


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





--- Comment #3 from Michal Nowak <mnowak at redhat.com>  2009-06-08 06:49:50 EDT ---
(In reply to comment #2)
> I've reviewed the package and it looks ok. There are only some minor and
> uncritical issues:

Thanks for the review, Christian. Good work.

> * rpmlint: TODO
> rpmlint SPECS/libxdg-basedir.spec SRPMS/libxdg-basedir-1.0.0-1.fc10.src.rpm
> RPMS/i386/libxdg-basedir-*
> libxdg-basedir.i386: W: no-documentation
> libxdg-basedir-devel.i386: W: no-documentation
> 4 packages and 1 specfiles checked; 0 errors, 2 warnings.
> 
> In general it is not a problem to have no documentation if
> a package doesn't provide any. ;-) However, in this specific case
> the package provides a doxygen API documentation (make doxygen-all).
> It would be good if it could be added to the devel package.

Now we have -docs sub-package with Doxygen generated documentation.

> * naming: OK
> - name matches upstream
> - spec file name matches package name
> 
> * sources: TODO
> - e32bcfa772fb57e8e1acdf9616a8d567  libxdg-basedir-1.0.0.tar.gz
> - sources matches upstream
> - Source0 tag ok
> - spectool -g  works
> - upstream version 1.0.1 was released a couple of weeks ago, please update to
> the new version (according to upstream's git repo it looks like a minor
> bug fix release)

Packed.

> * License: TODO
> - License MIT acceptable
> - License in spec file matches the actual license (MIT license header in
> libxdg-basedir-1.0.0/src/basedir.c )
> - No License file included, so there is no need to package it.
> - It would be better if upstream would provide a license file. According to the
> Review guidelines the packager is encouraged to query upstream to include it.
> However this will not block the review.

Encouraged :). You're in Cc.

> * spec file written in English and legible: OK
> 
> * compilation: OK
> - supports parallel build
> - RPM_OPT_FLAGS are correctly used
> - builds in mock (F10)
> - builds in koji:
> F10: https://koji.fedoraproject.org/koji/taskinfo?taskID=1394643
> F11: https://koji.fedoraproject.org/koji/taskinfo?taskID=1394648
> F12: https://koji.fedoraproject.org/koji/taskinfo?taskID=1397616
> 
> * BuildRequires: OK
> - no build requires are necessary
> 
> * locales handling: OK (n/a)
> 
> * ldconfig in %post and %postun: OK
> 
> * package owns all directories that it creates: TODO
> - %{_libdkir}/pkgconfig is created, but not owned by libxdg-basedir-devel
> - please add a "Requires: pkgconfig" to the devel package

Added.

> * no files listed twice in %files: OK
> 
> * file permissions: OK
> - %defattr used
> - actual permissions in packages ok
> 
> * %clean section: OK
> 
> * macro usage: OK
> 
> * code vs. content: OK (only code)
> 
> * large documentation into subpackage: OK (n/a)
> 
> * header files in -devel subpackage: OK
> 
> * static libraries in -static package: OK (n/a)
> 
> * package containing *.pc files must "Requires: pkgconfig": TODO (see above)
> 
> * *.so link in -devel package: OK
> 
> * - devel package requires base package using fully versioned dependency: OK
> 
> * packages must not contain *.la files: OK
> 
> * GUI applications must provide *.desktop file: OK (n/a)
> 
> * packages must not own files/dirs already owned by other packages: OK
> 
> * rm -rf $RPM_BUILD_ROOT at the beginning of %install: OK
> 
> * all filenames UTF-8: OK
> 
> * functional test: OK
> - compiling the provided test applications
> tests/testfind and tests/testdump
> - test apps compile successfully and the reported directory names seem to be
> meaningful

Added to %make check.

> * debuginfo sub-package: OK
> - non-empty
> - debuginfo file works together with gdb  

--
http://mnowak.fedorapeople.org/libxdg-basedir/libxdg-basedir.spec
http://mnowak.fedorapeople.org/libxdg-basedir/libxdg-basedir-1.0.1-1.fc11.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.