[Bug 595011] Review Request: <sshdfilter> <Filter for SSH ports>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=595011

--- Comment #5 from Rafael Aquini <aquini@xxxxxxxxx> 2010-05-23 16:22:51 EDT ---
David

Please, consider the following suggestions:

TIP: In order to avoid confusion to future reviewers, for every modification
you do to your package you should submit a new URL with your Spec and the new
SRPM built from it.


* Take a closer look to where you are installing sshdfilter SysV initscript:
>  install -p -m 755 -D etc/init.d/%{name} \
>                        %{buildroot}%{_sysconfdir}/init.d/%{name}

 - A better location would be %{buildroot}/%{_sysconfdir}/rc.d/init.d/%{name} 
 Let me show you why:
> [aquini@optiplex ~]$ ls -l /etc/init.d
> lrwxrwxrwx. 1 root root 11 2009-12-28 20:17 /etc/init.d -> rc.d/init.d


* Since you are using the macro %doc to 'INSTALL', 'todo' and
'iptables.example' under %files, you don't have  a real necessity to install
them under %install section (it's redundant). Actually it would be better if
you just use %doc to do the job of installing those documents.


* I'm not a legal expert, but I think you should use GPL+ as Licence: instead
of just GPL. 
 - http://fedoraproject.org/wiki/Packaging:Perl#License_tag


* To improve your changelogs, take a look at:
 - http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs


I will be looking closer to your approach to do changes in config files, to see
if I will be able to come up with some suggestion. Meanwhile, do not hesitate
in apply any improvements you want to sshdfilter.


Best regards

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]