Bug 177105
Summary: | Review Request: gnomeradio - Graphical FM-Tuner program | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dominik 'Rathann' Mierzejewski <dominik> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | j, mpeters, splinux, tcallawa |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-09-08 14:02:45 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: | 163779 |
Description
Dominik 'Rathann' Mierzejewski
2006-01-06 12:45:24 UTC
Here a couple of quick things I noticed that need to be corrected: 1. URL should be: http://www.wh-hms.uni-ulm.de/~mfcn/gnomeradio/ 2. SOURCE should be: http://www.wh-hms.uni-ulm.de/~mfcn/%{name}/packages/%{name}-%{version}.tar.gz 3. Your missing: Requires(post): scrollkeeper - refer to http://fedoraproject.org/wiki/ScriptletSnippets#head-3c9f517f0cd4aaabb369a8805226d85dc2f02793 4. Desktop file is not correct: refer to http://fedoraproject.org/wiki/PackagingGuidelines#head-37131c9c3cb4b69fdb1269f6e91fa9c413d2add1 5. Remove the %doc preceeding the help documents. 6. Missing Requires for GConf2: refer to http://fedoraproject.org/wiki/ScriptletSnippets#head-ff64cd482595764f672082d5a3b83e1fc22962e8 I would suggest going over the packaging information on the Wiki, since many of the problems with your spec can find the resolution there. http://fedoraproject.org/wiki/Extras Fixed all except 5. Why should I remove %doc? (In reply to comment #2) > Fixed all except 5. Why should I remove %doc? What benefit are you getting by keeping it? It's not like your using RPM to create a package-specific documentation directory during installation for these files. Also, I'm not aware of any other spec in FE that marks the help documentation with the %doc. Oh, glancing at your most recent changes to the spec file, you have an error in the %preun section. You need to replace [NAME] with %{name}. (In reply to comment #2) > Fixed all except 5. Why should I remove %doc? "%doc preceeding the help documents" - Perhaps a misunderstanding? The %doc tag was meant, not the %doc line before. PackageReviewGuidelines: - MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. This is what I read out of it: If help files are not present due to install with --excludedocs you will hopefully get an error message when pressing a help button. Thus it affects the runtime of your application. (In reply to comment #5) > "%doc preceeding the help documents" - Perhaps a misunderstanding? > The %doc tag was meant, not the %doc line before. Correct. I was referring to help documents (%{_datadir}/gnome/help/%{name}/), not the README, AUTHORS, etc. line. Speaking of which though, you also need to add the COPYING file. Just a suggestion - I would personally be a little more detailed on the %description: IE %description Gnomeradio is a FM-radio tuner for the GNOME desktop. It should work with every FM tuner card that is supported by video4linux. Remote controls are supported via LIRC-support. Gnomeradio can also record radio as a Wave or Ogg files. -=- That's from the homepage and modified a little bit. And I think you need a line break every 80 characters or so (which I didn't do above). Fixed all, changed the release tag down to 1, because it doesn't exist in FE yet. Thanks. * Missing BuildRequires gettext
* LIRC is also available in Extras (lirc-devel).
* rpmlint /home/qa/tmp/rpm/RPMS/gnomeradio-1.6-1.i386.rpm
W: gnomeradio conffile-without-noreplace-flag /etc/gconf/schemas/gnomeradio.sche
mas
It should not be marked %config.
* /usr/share/pixmaps/radio.png
This is pollution of the global pixmaps namespace and should be avoided.
The name "radio.png" is way too generic. The package stores "gnomeradio.png"
in the same directory, which creates a slightly better namespace. Suggest
renaming radio.png, also in the .desktop file.
* $ sudo rpm -ivh -p gnomeradio-1.6-1.i386.rpm
> Preparing... ########################################### [100%]
> 1:gnomeradio ########################################### [100%]
> gconfd-2: no process killed
Killing gconfd is no longer necessary in FC5, and in case it would be
necessary, make it shut up by redirecting its output. It is killed more
than once in the scriptlets, which is a guarantee for this warning.
* BuildRequires: gtk2-devel libgnomeui-devel
Only if you're into "optimising away" redundant build requirements,
notice that libgnomeui-devel requires gtk2-devel and glib2-devel already.
Else add a comment to make it explicit that _your_ package also depends
on the glib API directly (glib2-devel).
Fixed. APPROVED If you don't have a Fedora Extras account yet, continue at step 10 here, http://fedoraproject.org/wiki/Extras/Contributors and I'll sponsor you. *** Bug 188395 has been marked as a duplicate of this bug. *** *ping* Okay, everyone who reads this please listen: Hereby I announce that I withdraw my approval from 2006-04-04 and move this ticket back into the FE-NEW queue. Too much time has passed without action from the packager. Further, another package submission from a different packager has been rejected (bug 188395), but the software is still not in Fedora Extras after three months. This simply doesn't work and is a major hindrance, IMO. An approved package must not block other submissions for months. And it becomes an extra burden for me as a reviewer. Yes, you're totally right and I'm very sorry about the delay. It won't happen again. I've just jumped through the hoops of getting an account and pushed my first approved package (sysconftool). I promise to be more responsive from now on. Is the package back in comment #1 the one that should be reviewed? http://rpm.greysector.net/extras/gnomeradio.spec http://rpm.greysector.net/extras/gnomeradio-1.6-2.src.rpm This one. OK, this should be easy because of all of the other review work. It looks like your BuildRequires: pkgconfig is not strictly necessary on FC6 at least, but i doesn't harm anything. rpmlint only says: W: gnomeradio non-conffile-in-etc /etc/gconf/schemas/gnomeradio.schemas which in this case is bogus. It looks like /usr/share/omf is unowned; I don't see anything in the dependency chain that would create it. It seems that a whole pile of packages own it already, so it seems the expectation is that you should own it as well. * source files match upstream: 07b9d511f79e38f114af51cc7bfc014a gnomeradio-1.6.tar.gz * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text included in package. * latest version is being packaged. O BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (development, x86_64). * debuginfo package looks complete. * rpmlint has only ignorable complaints. * final provides and requires are sane: gnomeradio = 1.6-2.fc6 = /bin/sh GConf2 libICE.so.6()(64bit) libORBit-2.so.0()(64bit) libSM.so.6()(64bit) libX11.so.6()(64bit) libart_lgpl_2.so.2()(64bit) libatk-1.0.so.0()(64bit) libbonobo-2.so.0()(64bit) libbonobo-activation.so.4()(64bit) libbonoboui-2.so.0()(64bit) libcairo.so.2()(64bit) libgconf-2.so.4()(64bit) libgdk-x11-2.0.so.0()(64bit) libgdk_pixbuf-2.0.so.0()(64bit) libglib-2.0.so.0()(64bit) libgmodule-2.0.so.0()(64bit) libgnome-2.so.0()(64bit) libgnome-keyring.so.0()(64bit) libgnomecanvas-2.so.0()(64bit) libgnomeui-2.so.0()(64bit) libgnomevfs-2.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgthread-2.0.so.0()(64bit) libgtk-x11-2.0.so.0()(64bit) liblirc_client.so.0()(64bit) libpango-1.0.so.0()(64bit) libpangocairo-1.0.so.0()(64bit) libpangoft2-1.0.so.0()(64bit) libpopt.so.0()(64bit) libxml2.so.2()(64bit) libz.so.1()(64bit) scrollkeeper * %check is not present; no test suite upstream. * no shared libraries are added to the regular linker search paths. * package is not relocatable. ? owns the directories it creates (/usr/share/omf?) * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * scriptlets present are OK (gconf schema installation, scrollkeeper-update) * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings. * GUI app; desktop file looks to be installed properly. So it's just the /usr/share/omf thing that needs fixing or clarification. I don't think it is appropriate for this package to own /usr/share/omf. Maybe scrollkeeper should own it. Looks like scrollkeeper has been fixed in rawhide. That leaves FC5, but it's at best a minor issue and I don't think it should block this package. APPROVED Thanks for the review. I removed the pkgconfig BR, because it's redundant on FC5 as well. Package imported and built for devel, FC-5 branch requested. |