I cleaned up and improved the current net-firewall/arno-iptables-firewall ebuild. Changes include: - dependency updates - new initscript - cleaned up and simplified install - fix documentation path - whitespace and message cleanups I'll attach the updated ebuild and new initscript files. Reproducible: Always
Created attachment 344850 [details] arno-iptables-firewall-2.0.1d-r1.ebuild
Created attachment 344852 [details] arno-iptables-firewall.rc new initscript
I'd like to ask a proxy committer to review the previously attached ebuild and initscript files and provide feedback whether I should make further modifications to them. Thanks in advance! :)
Hi, Comments about the ebuild - Can we make it EAPI5 please? - Move S= below DEPEND please - Is /usr/share/arno-iptables-firewall/plugins the appropriate place to have executable plugins? I think /usr/libexec is preferred for this. I am not 100% sure though - You are mixing ewarn and elog rules in the postinst(). If you think all of the messages are important then use elog. Also consider using the readme.gentoo eclass for such messages.
Comments about the init script - Please drop the .rc suffix - "Single- & multi-homed": Why the extra '-' after Single?
Created attachment 348102 [details] updated ebuild file
Created attachment 348104 [details] arno-iptables-firewall-2.0.1d-r1.ebuild updated ebuild file
Created attachment 348106 [details] arno-iptables-firewall renamed initscript
Hi, thanks for the review, I've implemented the changes you mentioned and a bit of further cleanup: - change to EAPI 5 - move/quote S and DEPEND variables - install sourced/executable helper files to /usr/libexec - cleanup pkg_postinst and use readme.gentoo eclass - rename initscript - replace package's name with "${PN}" throughout the ebuild - remove accidental empty lines and a trailing slash Please find the updated files attached.
(In reply to comment #5) > - "Single- & multi-homed": Why the extra '-' after Single? It is taken directly from the upstream description and that hyphen is there to mean both single-homed and multi-homed. It's valid and used in my language so I haven't noticed it originally but during a conversation with ABCD and pchrist on #gentoo-dev-help it turned out to be valid in English too. At least according to them and http://changingminds.org/techniques/language/punctuation/hyphen.htm :)
(In reply to comment #9) > Hi, > > thanks for the review, I've implemented the changes you mentioned and a bit > of further cleanup: > > - change to EAPI 5 > - move/quote S and DEPEND variables > - install sourced/executable helper files to /usr/libexec > - cleanup pkg_postinst and use readme.gentoo eclass > - rename initscript > - replace package's name with "${PN}" throughout the ebuild > - remove accidental empty lines and a trailing slash > > Please find the updated files attached. Hi, good work. I believe the ebuild looks good enough for inclusion. I will take care of that.
minor thingie. newinitd "${FILESDIR}/${PN}" "${PN}" can be replaced by doinitd "${FILESDIR}/${PN}" because the one in files/ and the destination file have the same name.
Thanks. Committed +*arno-iptables-firewall-2.0.1d-r1 (18 May 2013) + + 18 May 2013; Markos Chandras <hwoarang@gentoo.org> + +arno-iptables-firewall-2.0.1d-r1.ebuild, +files/arno-iptables-firewall: + Cleanup, EAPI5, initscript and other fixes thanks to Ferenc Erki + <erkiferenc@gmail.com>. Bug #465114 + http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/net-firewall/arno-iptables-firewall/arno-iptables-firewall-2.0.1d-r1.ebuild?view=log http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/net-firewall/arno-iptables-firewall/files/arno-iptables-firewall?view=log
