From owner-svn-ports-all@FreeBSD.ORG  Tue Jan 27 07:11:57 2015
Return-Path: <owner-svn-ports-all@FreeBSD.ORG>
Delivered-To: svn-ports-all@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 1033)
 id E15C6194; Tue, 27 Jan 2015 07:11:57 +0000 (UTC)
Date: Tue, 27 Jan 2015 07:11:57 +0000
From: Alexey Dokuchaev <danfe@FreeBSD.org>
To: "Vanilla I. Shu" <vanilla@FreeBSD.org>
Subject: Re: svn commit: r377975 - in head/devel: . godot godot/files
Message-ID: <20150127071157.GA77865@FreeBSD.org>
References: <201501270345.t0R3jNnT067230@svn.freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201501270345.t0R3jNnT067230@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 <svn-ports-all.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-ports-all>,
 <mailto:svn-ports-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-ports-all/>
List-Post: <mailto:svn-ports-all@freebsd.org>
List-Help: <mailto:svn-ports-all-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-ports-all>,
 <mailto:svn-ports-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 27 Jan 2015 07:11:58 -0000

On Tue, Jan 27, 2015 at 03:45:23AM +0000, Vanilla I. Shu wrote:
> New Revision: 377975
> URL: https://svnweb.freebsd.org/changeset/ports/377975
> QAT: https://qat.redports.org/buildarchive/r377975/
> 
> [...]
> +GH_PROJECT=	godot

Redundant line, GH_PROJECT is set to PORTNAME by default.

> +USES=		scons pkgconfig compiler

Please keep USES values sorted (ascending).

> +USE_OPENSSL=	yes
> +# uses pkg-config to find ssl - pkg-config only finds port version
> +WITH_OPENSSL_PORT=	yes

These three lines make little sense and generally look like a hack.  How
hard is to patch it so it won't require WITH_OPENSSL_PORT?

> +MAKE_ARGS+=	platform=x11

Bogus usage of +=.

> +OPTIONS_DEFAULT=	EXAMPLES TOOLS

EXAMPLES is already included in default options set.

> +TOOLS_DESC=	Include development tools (IDE)
> +TOOLS_MAKE_ARGS_ON=	tools=yes
> +TOOLS_MAKE_ARGS_OFF=	tools=no target=release

This reads like "if built without tools, target != release (debug?)".

> +.if ${ARCH}==amd64 || ${ARCH}==powerpc64 || ${ARCH}==sparc64 || ${ARCH}==ia64

Why not just check for ${ARCH:M*64} if all these end in `64'?  There are
lots of examples in the ports for inspiration:

  $ find . -name Makefile | xargs grep 'ARCH:.*64'

> +BITSUF=	.64
> +.else
> +BITSUF=	.32
> +.endif

Maybe you would be able to come up with smart BITSUF=	.${...} assigment
instead of .if/.else/.endif. :)

> +post-patch:
> +	@${REINPLACE_CMD} -e 's|custom_build|${OPSYS}_Ports_build|' ${WRKSRC}/methods.py

Overly long line needs wrapping.

> +do-install:
> +	@cd ${WRKSRC}/bin && ${INSTALL_PROGRAM} godot.x11${BINSUFFIX} \
> +		${STAGEDIR}/${PREFIX}/bin/godot.x11${BINSUFFIX}

${INSTALL_PROGRAM} ${WRKSRC}/bin/godot.x11${BINSUFFIX} ... is shorter,
easier to read, and $cwd-agnostic.

> +	@${LN} ${STAGEDIR}/${PREFIX}/bin/godot.x11${BINSUFFIX} \
> +		${STAGEDIR}/${PREFIX}/bin/godot

Bogus symlink target (should be just "godot.x11${BINSUFFIX}").

./danfe