[Bug 460538] Review Request: ircd-ratbox - Ircd-ratbox is an advanced, stable and fast ircd

bugzilla at redhat.com bugzilla at redhat.com
Thu Aug 28 20:00:51 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=460538





--- Comment #3 from Lubomir Rintel <lkundrak at v3.sk>  2008-08-28 16:00:50 EDT ---
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?

-- 
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 package-review mailing list