From owner-svn-ports-head@FreeBSD.ORG  Mon Mar 18 03:47:49 2013
Return-Path: <owner-svn-ports-head@FreeBSD.ORG>
Delivered-To: svn-ports-head@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 1033)
 id E056D5E2; Mon, 18 Mar 2013 03:47:49 +0000 (UTC)
Date: Mon, 18 Mar 2013 03:47:49 +0000
From: Alexey Dokuchaev <danfe@FreeBSD.org>
To: Beech Rintoul <beech@FreeBSD.org>
Subject: Re: svn commit: r314491 - head/graphics/sam2p
Message-ID: <20130318034749.GA49703@FreeBSD.org>
References: <201303172049.r2HKnKMv046396@svn.freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
In-Reply-To: <201303172049.r2HKnKMv046396@svn.freebsd.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: svn-ports-head@freebsd.org, svn-ports-all@freebsd.org,
 ports-committers@freebsd.org
X-BeenThere: svn-ports-head@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: SVN commit messages for the ports tree for head
 <svn-ports-head.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-ports-head>,
 <mailto:svn-ports-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-ports-head>
List-Post: <mailto:svn-ports-head@freebsd.org>
List-Help: <mailto:svn-ports-head-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-ports-head>,
 <mailto:svn-ports-head-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Mon, 18 Mar 2013 03:47:49 -0000

On Sun, Mar 17, 2013 at 08:49:20PM +0000, Beech Rintoul wrote:
> New Revision: 314491
> URL: http://svnweb.freebsd.org/changeset/ports/314491

Not just replying to this particular commit, I occasionally notice similar
changes in other people's work.

> -	${INSTALL_PROGRAM} ${WRKSRC}/${PORTNAME} ${PREFIX}/bin
> -.if !defined(NOPORTDOCS)
> +	${INSTALL_PROGRAM} ${WRKSRC}/${PORTNAME} ${PREFIX}/bin/

Can we stop practices of 1) gratuitous, 2) confusing, 3) inconsistent
appending of slashes at the end of destination directory name?  They
are not helping anything here and just decrease STN ratio.  I guess I
could see a benefit of help install(1) to produce an error in special
cases when it can be hard to ensure target directory existence (think
function in a script), but this is not our case.

> +.if ${PORT_OPTIONS:MDOCS}
>  	@${MKDIR} ${DOCSDIR}
> -	${INSTALL_DATA} ${WRKSRC}/README ${DOCSDIR}
> +	${INSTALL_DATA} ${PORTDOCS:S|^|${WRKSRC}/|} ${DOCSDIR}

No slashes here, so what's wrong with ${PREFIX}/bin?  It's guaranteed to
exist since it part of the mtree.

> -	@(cd ${WRKSRC}/examples && ${COPYTREE_SHARE} . ${EXAMPLESDIR})
> +	@cd ${WRKSRC}/examples && ${COPYTREE_SHARE} \* ${EXAMPLESDIR}

What is this, seriously?  Dot looks natural, it is not subject to shell
expansion, it does not require escaping, it's safe, it works just fine.
Why changing it for no real reason?

I would like to remind us that we should be more critical towards what
maintainers submit, and not just blindly commit the diff if it passes
tinderbox sanity check.  Carefully educatiting maintainer also does not
hurt and helps everyone in the long run.

./danfe