On Mon, Jul 20, 2015 at 2:16 AM, Philip Oakley <philipoak...@iee.org> wrote:
> From: "Eric Sunshine" <sunsh...@sunshineco.com>
>> On Sun, Jul 19, 2015 at 4:08 PM, Philip Oakley <philipoak...@iee.org>
>> wrote:
>>> Keep the build clean of extraneous files if it is indeed clean.
>>> Otherwise leave the msvc-build-makedryerrors.txt file both as
>>> a flag for any CI system or for manual debugging.
>>>
>>> Note that the file will contain the new values of the GIT_VERSION
>>> and GITGUI_VERSION if they were generated by the make file. They
>>> are omitted if the release is tagged and indentically defined in
>>> their respective GIT_VERSION_GEN file DEF_VER variables.
>>>
>>> Signed-off-by: Philip Oakley <philipoak...@iee.org>
>>> ---
>>> +# test for an empty Errors file and remove it
>>> +for ($ErrsFile) {unlink $_ if (-f $_) && (!-s $_);}
>>
>> Why the 'for' loop?
>>
>> Also, if you're using the 'for' loop for the $_ side-effect, then why
>> not the simpler:
>
> It was cargo cult programming, with some Google searching to select between
> invocations. Most examples were looping through lists in scripts, hence the
> down select.
>
>> for ($ErrsFile) { unlink if -f && !-s; }
>
> A lot better. Will fix.
Although that works, I'm not sure that it's really all that desirable
due to the unnecessary and potentially confusing 'for' loop. I'd
probably just write it as:
unlink $ErrsFile if -f $ErrsFile && !-s _;
The lone '_' is magical[1] in that it re-uses the stat() information
from the -f rather than stat'ing $ErrsFile again. I'd also probably
replace !-s ("not non-zero size") with -z ("zero size"):
unlink $ErrsFile if -f $ErrsFile && -z _;
And, if you're using Perl 5.10 or later, you could use a little
syntactic sugar[1] and stack the file test operators up against one
another:
unlink $ErrsFile if -f -z $ErrsFile;
which is the equivalent of the above with the sugar removed.
[1]: http://perldoc.perl.org/functions/-X.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
