Bug 460538

Lubomir, please take a look in your free time at this package.

Dude this just plainly sucks.

No, seriously:

1.) You use Patch tag with an index and %patch macro without one:

Patch0:         ircd-ratbox-2.2.8-offbyone.patch
...
%patch -p1 -b .offbyone

Either replace "Patch9" with "Patch" or "%patch" with "%patch0"

2.) sysconfig/ircd has a redundant setting:

PIDFILE=/var/run/ircd.pid

The init script assigns a default value.
You may want to remove this from sysconfid/ircd

3.) Useless comment in init script:

# Note: pidfile is assumed to be created
# by ircd (config: server.pid-file).
# If not, uncomment 'pidof' line.

Really, there's no "'pidof' line"

4.) Patch status

You added a fairly long and sophisticated patch. You should send it upstream
accompany it with status comment.

5.) Useless comment in SPEC file:

#make %{?_smp_mflags} -C contrib

Get rid of that. Or build contrib stuff and eventually make a subpackage?

6.) Hardcoded path

--with-logdir=/var/log/ircd
...
mkdir -p $RPM_BUILD_ROOT/var/log ...

Replace with %{_localstatedir} (or %{_var}?)

7.) Own directories you create

Add %dir %{_sysconfdir}/ircd

8.) Add explaining comments to non-obvious commands:

mkdir -p ... $RPM_BUILD_ROOT/usr/local/ircd/

Why do you create a directory in /usr/local?

mv $RPM_BUILD_ROOT%{_datadir}/ircd-old/modules $RPM_BUILD_ROOT%{_datadir}/ircd/modules
rm -fr $RPM_BUILD_ROOT%{_datadir}/ircd-old

Why do you shuffle ircd-old around? What does it contain?

sed 's/-Werror//g' -i configure

Why do you strip this utterly useful compiler flag away?
What's the status of upstreaming this fix?

(In reply to comment #2)
> Dude this just plainly sucks.
I told you it's not really ready yet ;)

(In reply to comment #3)
> No, seriously:
> 
> 1.) You use Patch tag with an index and %patch macro without one:
> 
> Patch0:         ircd-ratbox-2.2.8-offbyone.patch
> ...
> %patch -p1 -b .offbyone
>
> Either replace "Patch9" with "Patch" or "%patch" with "%patch0"

Typo - interesting that it works :)
 
> 2.) sysconfig/ircd has a redundant setting:
> 
> PIDFILE=/var/run/ircd.pid
> 
> The init script assigns a default value.
> You may want to remove this from sysconfid/ircd

Right, done.

> 3.) Useless comment in init script:
> 
> # Note: pidfile is assumed to be created
> # by ircd (config: server.pid-file).
> # If not, uncomment 'pidof' line.
> 
> Really, there's no "'pidof' line"

I agree, I think the author meant "remove" by "uncomment".

> 4.) Patch status
> 
> You added a fairly long and sophisticated patch. You should send it upstream
> accompany it with status comment.

The patch will be send upstream if it's approved :)

> 5.) Useless comment in SPEC file:
> 
> #make %{?_smp_mflags} -C contrib
> 
> Get rid of that. Or build contrib stuff and eventually make a subpackage?

No need for contrib.

> 6.) Hardcoded path
> 
> --with-logdir=/var/log/ircd
> ...
> mkdir -p $RPM_BUILD_ROOT/var/log ...
> 
> Replace with %{_localstatedir} (or %{_var}?)

%{_localstatedir} is /usr/var, but I can make it _var :)

> 7.) Own directories you create
> 
> Add %dir %{_sysconfdir}/ircd

Done.

> 8.) Add explaining comments to non-obvious commands:
> 
> mkdir -p ... $RPM_BUILD_ROOT/usr/local/ircd/
> 
> Why do you create a directory in /usr/local?

Left that from debugging, latest SRPM uploaded.

> mv $RPM_BUILD_ROOT%{_datadir}/ircd-old/modules
> $RPM_BUILD_ROOT%{_datadir}/ircd/modules
> rm -fr $RPM_BUILD_ROOT%{_datadir}/ircd-old
> 
> Why do you shuffle ircd-old around? What does it contain?

Nothing, upstream Makefile just create this directory to take a backup of previous data in ircd, which is useless if you're building it in mock.

> sed 's/-Werror//g' -i configure
> 
> Why do you strip this utterly useful compiler flag away?
> What's the status of upstreaming this fix?

We don't really need to stop at every warning, causes unwanted things, you know :) However, as I've already told you, other distributions with older compiler are using it frequently (openSUSE). I'll mention it in my mail to upstream once this package will be approved. 

New SRPM is available, thank you.

Thanks, Well done!

APPROVED

Thanks to you.

New Package CVS Request
=======================
Package Name: ircd-ratbox
Short Description: Ircd-ratbox is an advanced, stable and fast ircd
Owners: mmahut
Branches: F-9, EL-5

cvs done.

Thank you!