[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