From owner-svn-ports-all@FreeBSD.ORG Tue Mar 31 14:40:34 2015 Return-Path: Delivered-To: svn-ports-all@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 1033) id AE64AEC; Tue, 31 Mar 2015 14:40:34 +0000 (UTC) Date: Tue, 31 Mar 2015 14:40:34 +0000 From: Alexey Dokuchaev To: Renato Botelho Subject: Re: svn commit: r382819 - in head/www: . e2guardian Message-ID: <20150331144034.GA31230@FreeBSD.org> References: <201503311413.t2VEDesm014206@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201503311413.t2VEDesm014206@svn.freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org X-BeenThere: svn-ports-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the ports tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Mar 2015 14:40:34 -0000 On Tue, Mar 31, 2015 at 02:13:40PM +0000, Renato Botelho wrote: > New Revision: 382819 > URL: https://svnweb.freebsd.org/changeset/ports/382819 > > [...] > Submitted by: Marcello Coutinho > > @@ -0,0 +1,99 @@ > +# Created by: Marcello Coutinho Would be nice to include creator's email in header as well, at least for consistency with majority of existing ports. > +MASTER_SITES= GH This line is redundant; in USE_GITHUB case, it will be added by bsd.sites.mk automatically (see the code around line 533). > +USE_GITHUB= yes > +GH_TAGNAME= ${PORTVERSION:S/^/v/} Why in-var substitution instead of simple and more readable v${PORTVERSION}? > +USES= pkgconfig iconv Mirordered USES (minor nit). > +HAS_CONFIGURE= yes HAS_CONFIGURE for GNU autoconf-based build system? Doesn't look right. > +USE_AUTOTOOLS= aclocal libtoolize autoheader automake autoconf > +ACLOCAL_ARGS= -I m4 > +AUTOMAKE_ARGS= --add-missing --copy --foreign Did you consider using USES=autoreconf instead of USE_AUTOTOOLS et al.? > +OPTIONS_DEFAULT=TRICKLE DOCS 1024 DOCS should not be part of OPTIONS_DEFAULT. > +APACHE_DESC= Enable Apache support for access denied page > +TRICKLE_DESC= Enable the trickle download manager > +CLISCAN_DESC= Enable support for CLI content scanners > +CLAMD_DESC= Enable ClamD AV content scanner > +ICAP_DESC= Enable ICAP AV content scanner support > +KAV_DESC= Enable Kaspersky AV support > +NTLM_DESC= Include NTLM authentication plugin > +DNS_DESC= Include DNS authetication plugin > +EMAIL_DESC= Enable e-mail reporting support We normally do not prepend the word "Enable" to option descriptions. See file Mk/bsd.options.desc.mk for examples of well-named options. > +OPTIONS_RADIO= DESCRIPTORS > +OPTIONS_RADIO_DESCRIPTORS= 1024 2048 4096 8192 Description for DESCRIPTORS (DESCRIPTORS_DESC) is missing. > +pre-configure: In-place patching is more naturally done in `post-patch' target. > + @${REINPLACE_CMD} -e 's|.lresolv||g' \ > + ${WRKSRC}/configure.ac I'm puzzled over this 's|.lresolv||g' part. This is the resulting diff I see: @@ -788,7 +788,7 @@ AC_MSG_RESULT(yes) dnsauth=true DNSAUTHSUPPORT="" - CXXFLAGS="${CXXFLAGS} -lresolv" + CXXFLAGS="${CXXFLAGS} " AC_DEFINE([PRT_DNSAUTH],[],[Define to enable DNS auth plugin]) fi], [ Can you elaborate on why 's|.lresolv||g' was used to achieve this? I also do not see the need for global modifier (g). Notice bogus trailing space before closing quote. ./danfe