Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: IceWM - Lightweight Window Manager. https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=222521 pertusus@xxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pertusus@xxxxxxx ------- Additional Comments From pertusus@xxxxxxx 2007-01-13 14:54 EST ------- Comments: * for me, tabs result in bad formatting * For man pages, I prefer something along %{_mandir}/man1/icewm.1* to catch different compression or no compression at all. * the themes are not packaged. Is it on purpose? * CXXFLAGS add -fpermissive -Wall -Wpointer-arith -Wconversion -Wwrite-strings -W inline -Woverloaded-virtual -W -fno-exceptions -fno-rtti is it right? * compilation warnings don't seem to be that harmfull to me, but may be worth reporting upstream (some system return flag may not be checked) * ldd -u -r on executables leads to many Unused direct dependencies. It is not very problematic, but it leads to unusefull dependencies on other libs sonames, which will lead you to unusefull rebuild when the soname change. Most of the time this is caused by pkgfonfig files not using corrrectly .private, or project not using pkgconfig it is this case for unusefull link on X libs in icewm, since icewm don't use pkgconfig for X libs. Needswork: * patch should be applied to lib/menu.in, not lib/menu * To have the prefix set correctly, configure and configure.in should be patched to set CONFIG_GNOME2_MENU_DIR instead of lib/menu(.in) CONFIG_GNOME2_MENU_DIR="${GNOME2_PREFIX}/share/gnome/vfolders/" should be CONFIG_GNOME2_MENU_DIR="${GNOME2_PREFIX}/share/gnome/desktop-directories/ " Then lib/menu.in could be patched to remove menuprog Gnome folder icewm-menu-gnome1 --list and use icewm-menu-gnome2 for kde folder too. * BR (BuildRequires) for kde-config is needed for CONFIG_KDE_MENU_DIR * things in the default menu should be removed or be Requires. In my opinion xterm could stay, but the remaining should go (especially mozilla which isn't in fedora). Mozilla could be replaced by htmlview, alternatively. * files in %{datadir}/icewm are not %config files, these are defaults. What is in --with-cfgdir are config file and there are none. My opinion is that %configure should have --with-cfgdir=%{_sysconfdir}/icewm otherwise /etc is hardcoded. * %{_sysconfdir}/icewm should be created and owned by the package * locales aren't handled right, it is covered in the guidelines http://fedoraproject.org/wiki/Packaging/Guidelines#head-8c605ebf8330f6d505f384e671986fa99a8f72ee * KDEDIR is taken from the environment, so /etc/profile.d/kde.sh should be sourced * gettext BuildRequires is missing * VERSION PLATFORMS in %doc seem unuseful to me -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review