Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=461131 --- Comment #32 from Patrice Dumas <pertusus@xxxxxxx> 2008-10-02 13:14:06 EDT --- I still have some comments. Instead of %define with_kde, please use bcond_with or bcond_without. The kdebase >= 3.0.0 BuildRequires is strange. Shouldn't it be kdebase-devel >= 3.0.0? And I don't really see the reason why it shouldn't also be set on fedora > 8? You should not repeat the summary in the %description. The checkout instructions are not enough, you should add the command that allows you to do the archive. The line with CFLAGS="$RPM_OPT_FLAGS" CXXFLAGS="$RPM_OPT_FLAGS" seems unuseful to me And you use $LOCALFLAGS but it is not set?? Remove the # Setup for parallel builds, use instead make %{?_smp_mflags} You should not do rm -rf $RPM_BUILD_DIR/%{name}-%{version} in %clean. You install icons, so it is likely that a post script is missing. Remove the Distribution tag. Why the BuildRequires autoconf, automake? Also remove gcc and gcc-c++ from BuildRequires, please, they are in the minimal buildroot. I also thought that zip was there too, but I am too lazy to check. openssl Requires should be picked up automatically. Some suggestions, feel free not to use these: * the TABs look bad in my editor, maybe you could either use only spaces or use tabs more consistently * The BuildRequires line that is very long could be split in 2 lines * remove from the description the text: SIM has countless features, many of them are listed at: http://sim-im.berlios.de/ since the URL is already a rpm tag. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review