[Bug 461131] Review Request: sim - Simple Instant Messenger
bugzilla at redhat.com
bugzilla at redhat.com
Fri Oct 3 20:30:57 UTC 2008
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 #33 from Pavel Alexeev <pahan at hubbitus.spb.su> 2008-10-03 16:30:56 EDT ---
Excuse me for the deadline. On this day I'm several times try build sim...
But I'm do not able tag package on branches except of devel:
[pasha at x-www F-9]$ cvs up -dPA
cvs update: Updating .
[pasha at x-www F-9]$ LANG=C make tag
cvs tag -c sim-0_9_5-0_6_20080923svn2261rev
ERROR: The tag sim-0_9_5-0_6_20080923svn2261rev is already applied on a
different branch
ERROR: You can not forcibly move tags between branches
sim-0_9_5-0_6_20080923svn2261rev:devel:hubbitus:1222767214
cvs tag: Pre-tag check failed
cvs [tag aborted]: correct the above errors first!
make: *** [tag] Error 1
I'm do it by docs -
http://fedoraproject.org/wiki/PackageMaintainers/Join#Tag_Or_Update_Your_Branches
and http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess
Please help me - what I do wrong?
(In reply to comment #32)
> I still have some comments.
>
> Instead of %define with_kde, please use bcond_with or bcond_without.
You think? I'm do not suppose this for buildtime config. But it seams as good
suggestion. I do that.
> 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?
I'm do not remember why I'm write it... May be you a right. But on fedora > 8
it should be kdebase3-devel >= 3.0.0. I add this BR.
And now I think over necessity of adding Requires: kdebase...
> You should not repeat the summary in the %description.
Ok, thanks.
> The checkout instructions are not enough, you should add the command
> that allows you to do the archive.
It seems excessive, but I'm add this.
> The line with
> CFLAGS="$RPM_OPT_FLAGS" CXXFLAGS="$RPM_OPT_FLAGS"
> seems unuseful to me
>
> And you use $LOCALFLAGS but it is not set??
Removed. But it do have sense.
> Remove the # Setup for parallel builds, use instead
> make %{?_smp_mflags}
Done.
> You should not do
> rm -rf $RPM_BUILD_DIR/%{name}-%{version}
> in %clean.
Ok.
> You install icons, so it is likely that a post script is missing.
What script you keep in mind?
> Remove the Distribution tag.
Removed. Does it make troubles?
> Why the BuildRequires autoconf, automake?
Because it is SVN build. We
> Also remove gcc and gcc-c++ from BuildRequires, please, they are in the
> minimal buildroot.
Ok.
> I also thought that zip was there too, but I am too lazy to check.
No, zip is not. See report about it before in this review.
> openssl Requires should be picked up automatically.
Where it is written?
> 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
In my editor (mcedit) my tab look correct. I'm assume convert tabs into spaces
for your editor you may by one command, like this: sed -i 's/\t/ /g' sim.spec
> * The BuildRequires line that is very long could be split in 2 lines
But it is not required, right?
> * 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.
No. This is "historical" text :)
I do not want remove them.
http://hubbitus.net.ru/rpm/Fedora9/sim/sim-0.9.5-0.7.20080923svn2261rev.src.rpm
--
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.
More information about the Fedora-package-review
mailing list