[Bug 608852] Review Request: epris - a dbus service to listen to music

bugzilla at redhat.com bugzilla at redhat.com
Thu Aug 5 11:38:30 UTC 2010


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=608852

--- Comment #9 from Michael Schwendt <mschwendt at gmail.com> 2010-08-05 07:38:28 EDT ---
Please do test your packages a little bit yourself, too. :(  Reviewers are
there to add another pair of eyes, but basically *you* are supposed to practise
RPM packaging in accordance with Fedora's Packaging and Reviewing Guidelines.


> %define gstreamer_version 0.10
> %define dbus-glib_version 0.70

> BuildRequires: gstreamer-devel > gstreamer_version
> BUildRequires: dbus-glib-devel > dbus-glib_version

> Requires:	gstreamer > gstreamer_version

This won't work. Did it even build and install?
Above you defined two macros, but below you didn't use them. So:

  BuildRequires: gstreamer-devel > %{gstreamer_version}
  BUildRequires: dbus-glib-devel > %{dbus-glib_version}
  Requires: gstreamer > %{gstreamer_version}

Further, using '>' and not '>=' is somewhat unclear. GStreamer is still in the
0.10 series for a long time, so would 0.10 be sufficient? Or does it strictly
need to be > 0.10? Notice that if the package %release value is not specified
in such a versioned dependency, it is left out of RPM version comparison, too.
For example, 0.10-2.fc14 would not be > 0.10


Btw, for safety reasons, notice:
https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define


> Obsoletes: epris < 0.2

A comment would be good here. There is no such "epris" provided in the Fedora
package collection. *This* package is called "epris". What's the reason for the
Obsoletes tag?


> %files
> %defattr(-,root,root,-)
> %doc

Absolutely no need to put an empty %doc there.

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


More information about the package-review mailing list