Spec URL: http://mmahut.fedorapeople.org/reviews/ircd-ratbox/ircd-ratbox.spec SRPM URL: http://mmahut.fedorapeople.org/reviews/ircd-ratbox/ircd-ratbox-2.2.8-1.fc8.src.rpm Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=790314 Description: ircd-ratbox is an advanced, stable, fast ircd. It is an evolution where ircd-hybrid left off around version 7-rc1. It supports the TS3 and TS5 protocols, and is used on EFnet and other IRC networks.
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!