Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Jul 2015 09:49:19 +0200
From:      Guido Falsi <madpilot@FreeBSD.org>
To:        Alexey Dokuchaev <danfe@FreeBSD.org>
Cc:        ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: svn commit: r393097 - in head/graphics: gphoto2 gphoto2/files libgphoto2 libgphoto2/files
Message-ID:  <55B8857F.3020904@FreeBSD.org>
In-Reply-To: <20150729071032.GA69069@FreeBSD.org>
References:  <201507282012.t6SKCkL5010769@repo.freebsd.org> <20150729071032.GA69069@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 07/29/15 09:10, Alexey Dokuchaev wrote:
> On Tue, Jul 28, 2015 at 08:12:46PM +0000, Guido Falsi wrote:
>> New Revision: 393097
>> URL: https://svnweb.freebsd.org/changeset/ports/393097
>>
>> Log:
>>   Update to 2.5.8
>>   
>>   PR:		201845
>>   Submitted by:	tkato432@yahoo.com
> 
> Guys,
> 
> Bear in mind that Kato's PRs often come with noise in the patches that
> should not be committed.  Always review their submission thoroughly.

I did, but this time some details slipped by me.

> 
>> @@ -10,6 +10,7 @@ MAINTAINER=	ports@FreeBSD.org
>>  COMMENT=	Command-line frontend to libgphoto2
>>  
>>  LICENSE=	GPLv2
>> +#LICENSE_FILE=	${WRKSRC}/COPYING
> 
> What was that supposed to mean, I wonder? :)

I had seen it, but since it causes no harm I thought it could stay. My
understanding is that LICENSE_FILE should be specified only if the
relevant file differs from the standard license. I would have removed
the line, but a commented line isn't a big deal. Anyway I'll clean this up.

> 
>>  post-install:
>> -	${INSTALL_DATA} ${WRKSRC}/NEWS ${STAGEDIR}${DOCSDIR}
>> +	(cd ${WRKSRC} && ${INSTALL_DATA} NEWS ${STAGEDIR}${DOCSDIR})
> 
> What this change was supposed to improve?  It's longer, it now takes two
> commands instead of one, it requires grouping, etc.  So how did it pass
> the review and got committed?

Oops, my fault, I did read it but I elaborated it like it was a
"COPYTREE", where the cd is actually needed.

Thanks for pointing this one out.

-- 
Guido Falsi <madpilot@FreeBSD.org>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55B8857F.3020904>