Subject: Re: [PATCH v2] t/gitweb-lib: Don't pass constant to decode_utf8

From: Jakub Narebski <[email-masked]>
Date: Thu, 21 Oct 2010 00:28:05 +0200

On Thu, 21 Oct 2010, Brian Gernhardt wrote:
> On Oct 20, 2010, at 3:05 PM, Jakub Narebski wrote:
>> On Wed, 20 Oct 2010, Brian Gernhardt wrote:
>> 
>>> Encode.pm started updating the string to decode in-place when a second
>>> argument is passed in version 2.40.  This causes 'decode_utf8("",
>>> Encode::FB_CROAK)' to die with a message like:
>> 
>> Very minor complaint: line break here makes it less readable.
>> 
>> Perhaps this?
>> 
>>  Encode.pm started updating the string to decode in-place when a second argument
>>  is passed in version 2.40.  This causes 'decode_utf8("", Encode::FB_CROAK)' to
>>  die with a message like:
> 
> The wrapping was automatically done by vim.  I'll try tweaking
> width/contents to make it flow better.

Note that it is just nitpicking: the commit message is quite fine as
it is.

>>> Changes since v1:
>>> - Use an explicitly empty variable instead of $_
>> 
>> ...which was undefined, not empty, I think.
> 
> It still served to test the two argument case, but I thought an explicit
> variable that exactly matched the previous call would be better. 

It is better, IMHO.

>>> - based against maint
>> 
>> Was there any difference versus being based against 'master'?
> 
> Whitespace in context lines.  The skip_all and test_done lines were
> re-indented at some point.  Minor, but I thought I'd mention it. 

O.K.

Thanks again.
-- 
Jakub Narebski
Poland