Spec URL: http://sailer.fedorapeople.org/vfrnav.spec SRPM URL: http://sailer.fedorapeople.org/vfrnav-0.3-2.fc11.src.rpm Description: vfrnav is a flight planning and navigation application for flights under visual flight rules.
Hello Thomas, this is not official review. I'm in need of sponsor but I can tell you what can be better or fixed. - your Release: doesn't match changelog version. - Group doesn't exist. You should use most appropriate Group available. less /usr/share/doc/rpm-*/GROUPS* - will give you all possible groups. - you dont need Require: gypsy. It will get pulled by gypsy-devel - gcc-c++ is not needed. Read here why: http://fedoraproject.org/wiki/Packaging/Guidelines - in %files section you can omit INSTALL. You are making installation so other users have no need to look into that file. - NEWS is empty. You can remove it. Same goes for README - package doesn't own %{_datadir}/%{name} dir. Include it to %files section with %dir. - You can speed up packaging by using asterix(*) to own all files in some directory. You can use this on %{_datadir}/%{name}/* to own all files inside. That's what I can see in fast - 10 minute - review.
Oops, forgot to tell you about licence. Its wrong. src file says GPLv2 or any later version. So you should change it from GPLv2 to GPLv2+
Thanks, Igor, for your comments! I've updated the package: Spec URL: http://sailer.fedorapeople.org/vfrnav.spec SRPM URL: http://sailer.fedorapeople.org/vfrnav-0.3-3.fc11.src.rpm
No problem. rpmlint says: vfrnav.spec:32: W: non-standard-group Applications/Navigation You should change that group too to something appropriate. You can simplify this part: %dir %{_datadir}/%{name} %{_datadir}/%{name}/vfrnav.png %{_datadir}/%{name}/bluetooth.png %{_datadir}/%{name}/BlankMap-World_gray.svg %{_datadir}/%{name}/dbeditor.glade %{_datadir}/%{name}/navigate.glade %{_datadir}/%{name}/routeedit.glade %{_datadir}/%{name}/prefs.glade %{_datadir}/%{name}/acftperformance.glade Like this: %dir %{_datadir}/%{name} %{_datadir}/%{name}/* - you are supplying desktop file but you are not installing it. http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files Desktop file supplied with vfrnav is weird - I never seen that kind of file. I would recommend you to make your own desktop file. All of this is explained at previous link. - You are installing icons for your desktop file but you never update icon cache. https://fedoraproject.org/wiki/PackagingDrafts/Icon_Cache
I've updated the package: Spec URL: http://sailer.fedorapeople.org/vfrnav.spec SRPM URL: http://sailer.fedorapeople.org/vfrnav-0.3-4.fc11.src.rpm (In reply to comment #4) > You should change that group too to something appropriate. Ugh. Oversaw that one. Fixed now. > Like this: > %dir %{_datadir}/%{name} > %{_datadir}/%{name}/* Yes I know I can use wildcards, however I'd like to get notified (by the unpackaged files check) if something changes there, so I would like to keep tne enumeration of all files in that directory. > - you are supplying desktop file but you are not installing it. > http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files Fixed (now validating them) > Desktop file supplied with vfrnav is weird - I never seen that kind of file. I > would recommend you to make your own desktop file. All of this is explained at > previous link. What is actually wierd about them? They conform to the spec as I read it... > - You are installing icons for your desktop file but you never update icon > cache. https://fedoraproject.org/wiki/PackagingDrafts/Icon_Cache Fixed, thanks!
(In reply to comment #5) > > Desktop file supplied with vfrnav is weird - I never seen that kind of file. I > > would recommend you to make your own desktop file. All of this is explained at > > previous link. > > What is actually wierd about them? They conform to the spec as I read it... Take a look at other desktop files. None of them has Encoding, Version, X-WIndow-Icon, X-Osso-Type. I never seen such desktop file. Best thing would be to wait for advanced packager to tell you is it ok or not.
(In reply to comment #4) > Like this: > %dir %{_datadir}/%{name} > %{_datadir}/%{name}/* And even shorter to %{_datadir}/%{name}/ (In reply to comment #5) > Yes I know I can use wildcards, however I'd like to get notified (by the > unpackaged files check) if something changes there, so I would like to keep tne > enumeration of all files in that directory. Well, IMO you just need to make sure you don't accidentally package unwanted files like leftovers from svn or cvs. I would use %dir %{_datadir}/%{name} %{_datadir}/%{name}/*.png %{_datadir}/%{name}/*.svg %{_datadir}/%{name}/*.glade But this is up to you. At least for desktop-file-validate I'd use wildcards because you might forget to validate a file otherwise. (In reply to comment #6) > Take a look at other desktop files. None of them has Encoding, Version, > X-WIndow-Icon, X-Osso-Type. Encoding should not be in the file, this is correct, but the rest of the file is acceptable. The version key is usually added during install by desktop-file-install and the other keys starting with X- are ok. Whenever you want to extend the freedesktop spec with custom keys you need to prefix them with X-, see http://standards.freedesktop.org/menu-spec/latest/ and especially http://standards.freedesktop.org/menu-spec/latest/ar01s03.html But there are other problems with the desktop files: - /usr/bin is hardcoded in the Exec tag. Ether remove it or walk over it with sed during %prep to make sure the path always works: sed -i 's!/usr/bin!%{_bindir}!' data/*.desktop - What is "X-Osso-Type=application/x-executable" meant for? Are these programs supposed to open a mimetype? I guess not, at least not x-executable, but if so, you need to follow https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database
(In reply to comment #7) > But this is up to you. At least for desktop-file-validate I'd use wildcards > because you might forget to validate a file otherwise. desktop-file-validate does not support multiple desktop files on its command line, so it needs to be enclosed in a for loop. > Encoding should not be in the file, this is correct, but the rest of the file Why should it not be? We seem to have lots of desktop files having an Encoding entry (lots of gnome-*, eg.). > But there are other problems with the desktop files: > - /usr/bin is hardcoded in the Exec tag. Ether remove it or walk over it with Removed the hardcoded path. > - What is "X-Osso-Type=application/x-executable" meant for? Are these programs > supposed to open a mimetype? I guess not, at least not x-executable, but if so, > you need to follow This is needed for maemo. On Fedora, it can go, so removed the X- tags. SRPM: http://sailer.fedorapeople.org/vfrnav-0.3-5.fc11.src.rpm SPEC: http://sailer.fedorapeople.org/vfrnav.spec
(In reply to comment #8) > Why should it not be? We seem to have lots of desktop files having an Encoding > entry (lots of gnome-*, eg.). Because it got removed from freedesktop specs a while ago. Now it is assumed now that files are UTF-8 by default. Yes, there are lots of old files left in Fedora and we should clean them up in the future, but we should not let anything enter Fedora when it's not following the specs.
Ok, so another update: SRPM: http://sailer.fedorapeople.org/vfrnav-0.3-6.fc11.src.rpm SPEC: http://sailer.fedorapeople.org/vfrnav.spec
Package fails to build due to wrong/missing BuildRequires: 1) BuildRequires wrong: libxml++ should be libxml++-devel 2) BuildRequires missing: zfstream-devel
Ok, thanks, fixed & updated: SRPM: http://sailer.fedorapeople.org/vfrnav-0.3-7.fc11.src.rpm SPEC: http://sailer.fedorapeople.org/vfrnav.spec Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1381322
Fedora review vfrnav-0.3-7.fc11.src.rpm 2009-05-29 $ rpmlint *.rpm vfrnav.spec vfrnav-utils.x86_64: W: no-documentation 4 packages and 1 specfiles checked; 0 errors, 1 warnings. * OK ? needs attention * rpmlint is OK * package is named according to the guidelines * specfile is named after the package * package is licensed under a Fedora approved licence (GPLv2+) * package license matches the license of the sources * license file (COPYING) is included as %doc * specfile is written in legible English * sources matches upstream $ md5sum vfrnav-0.3.tar.gz SRPM/vfrnav-0.3.tar.gz 674af747da8ea31642df815e909b2342 vfrnav-0.3.tar.gz 674af747da8ea31642df815e909b2342 SRPM/vfrnav-0.3.tar.gz * package builds in mock (Fedora 10) ? is the boost-devel BuildRequires needed? there are no include statements in the sources including any boost headers and there are no boost library dependencies in the built rpms configure does check for it though... ? directories not owned by package or its dependencies: 26x26 and 40x40 are very odd sized icons, neither /usr/share/icons/hicolor/26x26 nor /usr/share/icons/hicolor/40x40 are available in the hicolor-icon-theme package the subdirectories /usr/share/icons/hicolor/26x26/hildon and /usr/share/icons/hicolor/40x40/hildon are not owned by any package either - and a subdirectory called hildon is not standard either (not present in any of the directories for normal sized icons), is there a reason for not using something standard like apps instead? * no duplicate files * permissions are sane and %files has %defattr * %clean clears buildroot * spec file consistently uses macros * package contains code * %doc is not runtime essential ? is the utils subpackage useful/usable without the main package? it currently has no Requires on the main package * desktop files are processed with desktop-file-validate * package does not own other's directories * %install clears buildroot * installed filenames are UTF-8
(In reply to comment #13) Thanks for starting the review! > ? is the boost-devel BuildRequires needed? Indeed. It's not needed. > ? directories not owned by package or its dependencies: Convert the icons to more standard sizes (using ImageMagick), and then store them in the correct locations. Now at least plasma finds them. > ? is the utils subpackage useful/usable without the main package? > it currently has no Requires on the main package You could use the database conversion utilities in vfrnav-utils without vfrnav. But I added the Requires now, it doesn't hurt either. SRPM: http://sailer.fedorapeople.org/vfrnav-0.3-8.fc11.src.rpm SPEC: http://sailer.fedorapeople.org/vfrnav.spec Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1383258
Package approved.
Thanks for the review! New Package CVS Request ======================= Package Name: vfrnav Short Description: Flight Planning software for flights under Visual Flight Rules (VFR) Owners: sailer Branches: F-11 InitialCC:
CVS done.
vfrnav-0.3-8.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/vfrnav-0.3-8.fc11
vfrnav-0.3-8.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.