Spec URL: http://jfsaucier.fedorapeople.org/packages/radiotray.spec SRPM URL: http://jfsaucier.fedorapeople.org/packages/radiotray-0.5.1-1.fc12.src.rpm Description: Radio Tray is an online radio streaming player that runs on a Linux system tray. Its goal is to have the minimum interface possible, making it very straightforward to use. Radio Tray is not a full featured music player, there are plenty of excellent music players already. However, there was a need for a simple application with minimal interface just to listen to online radios. And that's the sole purpose of Radio Tray. rpmlint give no warning/error on spec, srpm and rpm. The koji scratch build are all fine for F-11, F-12, F-13 and rawhide : - http://koji.fedoraproject.org/koji/taskinfo?taskID=2115316 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2115320 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2115329 - http://koji.fedoraproject.org/koji/taskinfo?taskID=2115333 Here is what the two patches do : radiotray-0.5.1-fix-setup.patch : remove some redundant files radiotray-0.5.1-fix-desktop-file.patch : fix the syntax of the desktop file to comply with the guidelines. Thank you!
hey, a quick look. ## The python_sitelib definition could be changed to http://fedoraproject.org/wiki/Packaging/Python#Macros ## The build section could use CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build This way the correct flags are used. ## line 37 could use %{__python} instead of python (optional) ## %{python_sitelib}/radiotray-0.5.1-py?.?.egg-info would help not break the package in case of py version changes? No major issues, I'll do a complete review within the coming week hopefully. regards, Ankur
Spec URL: http://jfsaucier.fedorapeople.org/packages/radiotray.spec SRPM URL: http://jfsaucier.fedorapeople.org/packages/radiotray-0.5.1-2.fc12.src.rpm Thank you for your suggestions. rpmlint is still silent after the changes and build fine in mock. > ## The python_sitelib definition could be changed to Done, I did not notice this change since F-13, thanks for pointing this to me! > ## The build section could use > CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build Done. > ## line 37 could use %{__python} instead of python (optional) Done. > ## %{python_sitelib}/radiotray-0.5.1-py?.?.egg-info would help not break the > package in case of py version changes? Done. Thanks for pointing out some python stuff. I don't have many experience packaging python applications and with the new multiple python version feature in F-13, I think it's important to make these changes.
review: + OK ? issue - NA + Package meets naming and packaging guidelines + Spec file matches base package name. + Spec has consistant macro usage. + Meets Packaging Guidelines. + License ? License field in spec matches + License file included in package + Spec in American English + Spec is legible. + Sources match upstream md5sum: [Ankur@localhost SPECS]$ md5sum radiotray-0.5.1.tar.gz 9811f8145108784e8e515b66fdaa6f05 radiotray-0.5.1.tar.gz [Ankur@localhost SPECS]$ md5sum ../SOURCES/radiotray-0.5.1.tar.gz 9811f8145108784e8e515b66fdaa6f05 ../SOURCES/radiotray-0.5.1.tar.gz - Package needs ExcludeArch + BuildRequires correct + Spec handles locales/find_lang - Package is relocatable and has a reason to be. + Package has %defattr and permissions on files is good. + Package has a correct %clean section. + Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) + Package is code or permissible content. - Doc subpackage needed/used. + Packages %doc files don't affect runtime. - Headers/static libs in -devel subpackage. - Spec has needed ldconfig in post and postun - .pc files in -devel subpackage/requires pkgconfig - .so files in -devel subpackage. - -devel package Requires: %{name} = %{version}-%{release} - .la files are removed. + Package is a GUI app and has a .desktop file + Package compiles and builds on at least one arch. + Package has no duplicate files in %files. + Package doesn't own any directories other packages own. + Package owns all the directories it creates. + No rpmlint output. NOT CHECKED YET - final provides and requires are sane: (include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done SHOULD Items: + Should build in mock.: builds in koji - Should build on all supported archs + Should function as described. - Should have sane scriptlets. - Should have subpackages require base package with fully versioned depend. + Should have dist tag + Should package latest version - check for outstanding bugs on package. (For core merge reviews) Issues: 1. please check the license, the COPYING and source files feature a GPLv1+ 2. please comment the spec file and add reasons for patching in there too. 3. why are explicit requires used in the spec? Please add comments justifying them http://fedoraproject.org/wiki/PackagingGuidelines#Explicit_Requires Please correct the above, I'll complete the review once they're taken care of. regards, Ankur
I have a question before posting a new package : > 1. please check the license, the COPYING and source files feature a GPLv1+ Done. You are right, the website mention GPL and I assume GPLv3... It's clearly a GPLv1+ in the COPYING file. > 2. please comment the spec file and add reasons for patching in there too. Done. I added the comments to patches and 1-2 other places in the SPEC file. > 3. why are explicit requires used in the spec? Please add comments justifying them This is my question. I don't see the explicit requires in my SPEC file. I think I don't have any but maybe it's something I don't know. Can you enlight me on this? Thanks for the review.
(In reply to comment #4) hey, > > > 3. why are explicit requires used in the spec? Please add comments justifying them > > This is my question. I don't see the explicit requires in my SPEC file. I think > I don't have any but maybe it's something I don't know. Can you enlight me on > this? > > > Thanks for the review. That would be line 18. Requires: gstreamer-python pygtk2 pygobject2 python-lxml python-inotify regards, Ankur
> It's clearly a GPLv1+ in the COPYING file. The COPYING file contains GPL version 1. The 1+ comes from the .py file headers. The Fedora license tag for that is GPL+ not GPLv1+.
I removed the pygobject2 from the Requires because pygtk2 require it automatically. Is it what you mean by explicit requires? If yes, I am ready to provide a new SPEC and SRPM. Excuse me if it's a dump question but I always though that explicit require was more like : libfubar >= 0:1.2.3-7. Thanks!
The guidelines just ask you to add a comment in the spec file and explain why explicit dependencies are needed. In this case they are needed, because there are no automatic dependencies for the needed Python modules. [...] With regard to eliminating redundant Requires (e.g. pygobject2 <-- pygtk2, although radiotray directly imports the gobject module, too), it's not necessary to trim such Requires, and in other cases it may even bear a risk.
(In reply to comment #8) > The guidelines just ask you to add a comment in the spec file and explain why > explicit dependencies are needed. In this case they are needed, because there > are no automatic dependencies for the needed Python modules. > > [...] > > With regard to eliminating redundant Requires (e.g. pygobject2 <-- pygtk2, > although radiotray directly imports the gobject module, too), it's not > necessary to trim such Requires, and in other cases it may even bear a risk. Thanks for pitching in Michael :) Jean, please submit the updated spec and srpm and we can get this one approved quick. :) Ankur
Spec URL: http://jfsaucier.fedorapeople.org/packages/radiotray.spec SRPM URL: http://jfsaucier.fedorapeople.org/packages/radiotray-0.5.1-3.fc12.src.rpm Hi, Thanks for the review and pardon me for taking so much time to post an update, I was in training for the entire week. Everything should be fixed now! rpmlint is still silent after the changes and the package build fine in mock.
hey, [ankur@mether SPECS]$ rpmlint radiotray.spec /var/lib/mock/fedora-rawhide-i386/result/*.rpm radiotray.spec: W: invalid-url Source0: http://downloads.sourceforge.net/radiotray/radiotray-0.5.1.tar.gz <urlopen error timed out> radiotray.noarch: W: no-manual-page-for-binary radiotray radiotray.src: W: invalid-url Source0: http://downloads.sourceforge.net/radiotray/radiotray-0.5.1.tar.gz <urlopen error timed out> 2 packages and 1 specfiles checked; 0 errors, 3 warnings. Only one worth looking at is the second one. I'd suggest writing a man page for the package. You could also try to make this work on python3. Apart from this, it looks good :) XXX APPROVED XXX
hey, [ankur@ SPECS]$ rpmlint radiotray.spec /var/lib/mock/fedora-rawhide-i386/result/*.rpm radiotray.spec: W: invalid-url Source0: http://downloads.sourceforge.net/radiotray/radiotray-0.5.1.tar.gz <urlopen error timed out> radiotray.noarch: W: no-manual-page-for-binary radiotray radiotray.src: W: invalid-url Source0: http://downloads.sourceforge.net/radiotray/radiotray-0.5.1.tar.gz <urlopen error timed out> 2 packages and 1 specfiles checked; 0 errors, 3 warnings. I'd advise you to write up a small man page if you can. Also, have a look if you can append python3 support to the spec. Rest looks cool. XXX APPROVED XXX
Thank you for the review! I will try to work with upstream for the things you mention in the near future. New Package CVS Request ======================= Package Name: radiotray Short Description: Radio Tray is a streaming player for listening to online radios Owners: jfsaucier Branches: F-11 F-12 F-13 InitialCC:
CVS done (by process-cvs-requests.py). We no longer allow f11 branches with the release of f13.
radiotray-0.5.1-3.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/radiotray-0.5.1-3.fc13
radiotray-0.5.1-3.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/radiotray-0.5.1-3.fc12
radiotray-0.5.1-3.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update radiotray'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/radiotray-0.5.1-3.fc13
radiotray-0.5.1-3.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update radiotray'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/radiotray-0.5.1-3.fc12
radiotray-0.5.1-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
radiotray-0.5.1-3.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.