[Bug 461131] Review Request: sim - Simple Instant Messenger

[bugzilla at redhat.com]

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 at free.fr>  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.