Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.7.1-0.nixa.2.src.rpm Description: Tasty Menu is a KMenu replacement aiming to provide the maximum usability, or at least to be a testbed for usability concepts and ideas for a future KMenu replacement. The left part of the menu is very similar to the Novell idea (http://www.novell.com/products/desktop/preview.html); you have a search box that is always selected when the menu is opened (the search results are displayed in the leftmost listview), followed by a combobox that decides what appears in the following listview: favorite applications (default), most used applications, recently used applications or recent documents. The right part contains the whole KMenu and takes the aspect from KBFX (http://www.kbfx.org); the middle column contains the top level categorization (plus in the current KMenu arrangment the Control Center, Home folder and Find Files, but I think only categories should be present). and the left-most listview contains the content of the category currently selected in the middle column. I think in this way even if it has the same number of items it "appears" less huge than with a popup menu/submenu structure. Every item has two rows, for the name and for the description, in order to make it more informative. On each selected item an action icon appears on the right; at the moment they are "add bookmark" on application icons and "remove bookmark" on favorite apps list. The bottom buttons are the usual switch user, lock session and logout. At first I didn't want to put them, I thought that these functions should be delegated to another applet (like http://kde-apps.org/content/show.php?content=26150), but it seems that this doesn't work very well in practice. The left-most button contains the user name and icon, and clicking on it opens the Profile Editor. I know it seems silly, but it's only an experiment, probably it will be merged with the switch user button.
- The release tag is incorrect. Use a single digit unless to do something else. - The description tag is very long; I would suggest reducing it to one or two paragraphs. - According to the list of files, a static library is included in the package. Take a look at this: http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4bcce24cb711266557ad7
Kelly needs sponsor. As a KDE user, I would like to participate to the review of that package, but there is a tradition in Fedora that the review of the first package of a new contributor is done by the sponsor. I can however give several remarks. - I second remarks of comment #1 about the release and descriptions tags. However, the file list shows no static lib. The .la file is a libtool auxiliary file, needed by the dlopen implementation used in KDE3. - The tarball shipped in the src.rpm is the same as the upstream one. - Read the guidelines about the use of desktop-file-install: http://fedoraproject.org/wiki/Packaging/Guidelines#head-d559ee7363418a5840ce63090c608c991cd39ce6 - There is something special to do with the HTML documentation. See the spec file of kdebluetooth (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=235203) for an example. Rex, I thought there was a project for Packaging/KDE in the Wiki. Do you have a draft somewhere? I am quite disappointed not to be able to link a wiki page about the KDE HTML doc packaging.
(In reply to comment #2) > - I second remarks of comment #1 about the release and descriptions tags. As regards the release tag, read the guidelines: http://fedoraproject.org/wiki/Packaging/NamingGuidelines It would also nice that you use a disttag: http://fedoraproject.org/wiki/Packaging/DistTag
> - Read the guidelines about the use of desktop-file-install: > http://fedoraproject.org/wiki/Packaging/Guidelines#head-d559ee7363418a5840ce63090c608c991cd39ce6 I was actually wondering about this one; do I still have to use desktop-file-install, even though it's installing the .desktop file to Kicker and not to the menu proper?
Okay, now I'm confused. I changed the Release tag to 1%{?dist} as suggested in the DistTag guide, but instead of calling the package -1.fcX.i386.rpm, it's calling it -1fcX.i386.rpm.
> do I still have to use desktop-file-install... no. Re: %{?dist} usage, 1%{?dist} is correct. and dist *should* = .fcX (not fcX).
Okay, since I'm not sure what the problem with the dist naming is, I've uploaded the updated spec and source files. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.7.1-1fc6.src.rpm
Kelly, I can sponser you, however I really prefer it if you can log onto EFNet IRC #fedora-devel as communication this way is much faster and easier. My nick there is XulChris. Some initial comments on your spec file: - Release tag needs a dist tag, for example: Release: 2%{?dist} Do not use 0.nixa. stuff, also remove dist tag stuff from your changelog comments, it should just be: * Fri Apr 27 2007 Kelly Miller <lightsolphoenix> 0.7.1-2 - The URL tag should just be: http://www.notmart.org - Your BR lists redundant packages, change it to: BuildRequires: qt-devel >= 3.0.0, kdelibs-devel >= 3.0.0 - I would recommend just using the first paragraph for summary - Change %files to %{_docdir}/HTML/en/tastymenu so that you own the directory, do not bother listing each file under this directory, the directory is all you need to list, anything under it gets picked up automatically.
oops, sorry for lack of sleep last night I mentioned the wrong IRC network :) #fedora-devel is on freenode.net, not EFNet Also, please shorten the Summary tag, all you need is: Summary: KMenu replacement
> - Your BR lists redundant packages, change it to: > BuildRequires: qt-devel >= 3.0.0, kdelibs-devel >= 3.0.0 Actually, that was my original BuildRequires list, but when I tested the build using mock, the system complained about qt and kdelibs not being there. When I added them to the BuildRequires list, the package compiled under mock.
Kelly: qt-devel and kdelibs-devel will bring in those other packages automatically. I imagine you made some other mistake previously. I tested it myself and it works here just fine. If you are getting build failures in mock check your build.log and root.log. The root.log should show those packages being added. Also, I tested this build on a 64bit system and there are 64bit rpath problems, please add this to your %prep section: # Avoid lib64 rpaths sed -i -e 's|"/lib /usr/lib|"/%{_lib} %{_libdir}|' configure Also, please run rpmlint on your packages after they are built, there are some rpmlint output which can be easily fixed, for example: W: tastymenu mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 46) W: tastymenu dangling-relative-symlink /usr/share/doc/HTML/en/tastymenu/common ../common W: tastymenu incoherent-version-in-changelog 1.fc6 0.7.1-1.fc6 E: tastymenu-debuginfo script-without-shebang /usr/src/debug/tastymenu-0.7.1/src/misc.cpp Rex: I'm not sure about the following rpmlint error, is this typical of KDE apps? E: tastymenu invalid-soname /usr/lib64/tastymenu_panelapplet.so tastymenu_panelapplet.so
W: tastymenu dangling-relative-symlink /usr/share/doc/HTML/en/tastymenu/common ../common Actually, this can be ignored. Also, in comment #8 I said: - I would recommend just using the first paragraph for summary I actually meant the description tag, not summary tag.
Nice work! tastymenu-0.7.1-1fc6.src.rpm seems a far better packaging than the previous one. There is still the issue of ${_libdir}/ tastymenu_panelapplet.so And your BuidlRequires are a little too long. "BuildRequires: qt-devel, kdelibs-devel" should be enough, even though it is not a critical issue. I have not looked that the compilation itself, but why kdebase is build-required? I copy-paste the issues found by rpmlint below: lrineau@tsetse ~/RPM $ rpmlint SRPMS/tastymenu-0.7.1-1.amoi_rineau.src.rpm W: tastymenu mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 42) lrineau@tsetse ~/RPM $ rpmlint tastymenu W: tastymenu incoherent-version-in-changelog 1 0.7.1-1.amoi_rineau E: tastymenu invalid-soname /usr/lib/tastymenu_panelapplet.so tastymenu_panelapplet.so W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZTI8KService W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZTI13KServiceGroup W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN8KService20serviceByDesktopPathERK7QString W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN13KServiceGroup7entriesEbbbb W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN9KDirWatch7addFileERK7QString W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN4KRun10runCommandE7QString W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZNK9KFileItem4timeEjRb W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN13KServiceGroup4rootEv W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN13KServiceGroup5groupERK7QString W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN15KRecentDocument15recentDocumentsEv W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN13KServiceGroup10childCountEv W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN10KDirListerC1Eb W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN11KIconButtonC1EP7QWidgetPKc W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN18KDEDesktopMimeType3runERK4KURLb W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZNK8KService6menuIdEv W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZNK8KService6pixmapEN5KIcon5GroupEiiP7QString W: tastymenu undefined-non-weak-symbol /usr/lib/tastymenu_panelapplet.so _ZN9KDirWatchC1EP7QObjectPKc W: tastymenu unused-direct-shlib-dependency /usr/lib/tastymenu_panelapplet.so /lib/libm.so.6
indeed, there seems to be some linking problems. Also, I get a segfault when trying to configure tastymenu.
it appears this needs to be linked against libkio as well. Kelly, you might want to contact upstream author and ask him about this. It seems tastymenu uses KServiceGroup class which is part of the kio library, but he does not link against libkio. I've fixed this by adding this line after %configure: sed -i -e 's|$(LIB_KDEUI)|$(LIB_KDEUI) $(LIB_KIO)|' src/Makefile The unused-direct-shlib-dependency warnings (I get two of them) are caused by libraries brought in through libtool from this line: postdeps="-lstdc++ -lm -lgcc_s -lc -lgcc_s" This cannot be worked-around by using Fedora's libtool because it does the same thing. I'm not sure how to avoid this warning other than adding this after %configure: sed -i -e 's|"-lstdc++ -lm -lgcc_s -lc -lgcc_s|"-lstdc++ -lc|' libtool But I'm not entirely sure what is going on here and if this is the proper fix. It might be okey to just ignore those warnings.
After a bunch of adjustments, here's the updated file list: Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.7.1-4fc6.src.rpm
Please remove the qt-devel from BuildRequires, this is redundant and is picked up by kdelibs-devel. There is also a new version out. I'm not sure yet what to do with the .so name. Perhaps Rex might know more. I found another bug with new subfolders not showing up. For example if you install an application that creates a new subfolder, the application will not show up in tastymenu, but shows up in kmenu.
Done, and I also updated the package to version 0.8. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.8-1fc6.src.rpm
Kelly, to fix the last remaining rpmlint issue, please compile with: %configure --disable-rpath --libdir=%{_libdir}/kde3 and changes your %files to read: %{_libdir}/kde3/*
Done. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.8-3fc6.src.rpm
Another quick update; I added the script to remove the unnecessary .la files from the package. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.8-4fc6.src.rpm
> I added the script to remove the unnecessary .la files Most kde apps really *do* need those for proper function at runtime.
Okay; I wasn't sure, but I removed the line again. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.8-5fc6.src.rpm
==== REVIEW CHECKLIST ==== - rpmlint output: W: tastymenu dangling-relative-symlink /usr/share/doc/HTML/en/tastymenu/common ../common This is okay, package requires kdelibs which owns this file. - package named according to package naming guidelines - spec filename matches %{name} - package meets packaging guidelines - licensed with open source compatible license - license field matches actual license - license file included in %doc - spec written in American english - spec legible - sources match upstream d58492c17fe97615e912d28399fce2ef tastymenu-0.8.tar.bz2 - successfully compiles and builds on FC6 x86_64 - all build dependencies listed in BR - locales handled properly - no shared library in default path - package not relocatable - package owns all directories it creates X package Requires packages to bring in all other used directories - no duplicates in %files - file permissions set properly - contains proper %clean section - macro usage consistent - contains code - no large documentation - files in %doc do not affect runtime - no header files - no static libraries - no pkgconfig files - no library with suffix - no need for devel subpackage - package contains libtool archive This is a KDE package which is an exception to the .la requirement - contains proper .desktop file - package does not own files or directories owned by other packages - contains proper %install - all filenames valid utf-8 ==== MUST FIX ===== - Add: Requires: kdebase This brings in the /usr/share/apps/kicker/applets/ directory.
Done. Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/tastymenu.spec SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/tastymenu-0.8-6fc6.src.rpm
Hi Kelly, can you do a review of someone else's package and link to the bug # here? Want to make sure you know the review procedures and can do a capable review. :) Thanks.
I can try, but I'm not a real fantastic reviewer of other people's work.
just go through the items listed here: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines and mark off the items as you check them. see comment #24 for an example. click on the FE-NEEDSPONSOR link below to find a list of packages that probably have a couple errors in them. :) I try to do atleast 1 review for every package I submit. The more you do the better you get at it. Thanks.
Done. https://bugzilla.redhat.com/bugzilla/process_bug.cgi#c8
I was hoping to see you at the KDE SIG meeting today, pop in IRC when you have some free time and msg me and we'll get you set up with sponsership! :)
All MUST FIX items fixed, package looks good. **** APPROVED **** Feel free to ping me on IRC if you have any questions.
New Package CVS Request ======================= Package Name: tastymenu Short Description: KMenu replacement Owners: lightsolphoenix Branches: FC-6 InitialCC: lightsolphoenix
cvs done
Package Change Request ====================== Package Name: tastymenu New Branches: FC-7
cvs done Please note the Fedora 7 branch is F-7 not FC-7
As far as I know, this bug should be closed.
Yeah. My first reaction was "It's not closed?"