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: ndesk-dbus-glib - glib mainloop integration for ndesk-dbus Alias: ndesk-dbus-glib https://bugzilla.redhat.com/show_bug.cgi?id=245492 peter@xxxxxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO Flag| |needinfo?(david@xxxxxxxxxxxx | |t) ------- Additional Comments From peter@xxxxxxxxxxxxxxxx 2007-11-01 23:18 EST ------- Okey dokey....sorry about the lateness of this. Here we go! == GOOD == + Naming and version/release are appropriate. + Source tarball matches that of upstream: 319d423fa3dfe8bfddba69e2b7c58df8 dbus-sharp-glib-0.3-upstream.tar.gz 319d423fa3dfe8bfddba69e2b7c58df8 dbus-sharp-glib-0.3-srpm.tar.gz + License (MIT) is Fedora-acceptable and matches the licenses of the source files. + Spec file is written in American English, and is legible. + Builds fine in mock (F7 and Development, x86_64); all BuildRequires are listed without duplicates or repeats via dependencies. + ExcludeArch bug filed as appropriate (PPC64) + Final file/directory ownership is good, and does not conflict with system packages. No duplicate files are listed. + Files listed properly with an appropriate %defattr(...) line. + A %clean section exists and includes only "rm -rf $RPM_BUILD_ROOT" as required. + Macro usage is consistent. + Package contains code. + pkgconfig data (.pc file) is in a devel subpackage as required; and the devel subpackage properly hardcodes a dependency on pkgconfig. + devel subpackage properly hardcodes a dependency on its parent (ndesk-dbus-glib) of the same Version/Release. + The buildroot is properly cleaned at the beginning of %install (via "rm -rf $RPM_BUILD_ROOT"). + All filenames are valid UTF-8. == MINOR == (1) The "Url" tag in your spec file should be all-uppercase: "URL: ..." (2) rpmlint complains about the source RPM, though the binary > ndesk-dbus-glib.src: W: mixed-use-of-spaces-and-tabs (spaces: line 17, tab: line 1) Just an inconsistency between using spaces and tabs for indenting. Choose one or the other and stick to it, please. :) (3) Your spec contains: > Source1: include.mk > Patch0: build-fix.patch Please prefix any extra files with %{name}, so that file conflicts are kept to a minimum when installing multiple source RPMs to the same build tree. (4) You're using %{?buildsubdir} in the various MONO_SHARED_DIR settings. Is that really needed? There seems to be no definition for it in the standard rpm macros. (Then again, this was also in the ndesk-dbus package, so if you remove it from here, please remove it in that package as well to keep consistent.) (5) While understandable, the ndesk-dbus dependency is not needed, as RPM Mono-handling is sane enough that it registers the library as a mono() provides, and knows that this is linked to it so uses that for it's dependency. During their builds, the following are output: ndesk-dbus: > Provides: mono(NDesk.DBus) = 1.0.0.0 ndesk-dbus-glib: > Provides: mono(NDesk.DBus.GLib) = 1.0.0.0 > Requires: mono(NDesk.DBus) = 1.0.0.0 mono(mscorlib) = 2.0.0.0 ndesk-dbus Unless another alternate package also provides this, you should not need to manually specify the ndesk-dbus dep. === BLOCKER === - Since the upstream source tarball includes a copy of the license text (COPYING), please add to the %doc of the package. == NOT APPLICABLE == * Includes no i18n, so %find_lang is not needed. * Does not install shared libs; /sbin/ldconfig invocations are not needed. * Package is not relocatable. * Package does not need a doc subpackage, since no documentation is included. * No %doc files are listed; thus nothing in runtime breaks if they are not present. * No headers or static libraries; and no native-code shared libraries. * Package contains no .la (libtool) or GUI stuff. Also, would you please bump the package to 0.4.1 (available from their website)? Once we can filter through these small issues, it should be golden. :) -- 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, or are watching someone who is. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review