Re: [PATCH] Make git-clean a builtin

[Junio C Hamano <gitster@pobox.com>]

Shawn Bohrer <shawn.bohrer@gmail.com> writes:

> On Mon, Nov 05, 2007 at 01:14:32PM -0800, Junio C Hamano wrote:
>> Shawn Bohrer <shawn.bohrer@gmail.com> writes:
>> [...]
>> > +static int disabled = 1;
>> 
>> This means we are committed to make clean.requireForce default
>> to true, which is fine by me.  I need to warn the users about
>> this early.
>
> Actually I don't care either way, but in my last rebase on next this
> change was already made to git-clean.sh so I adjusted accordingly.

Oh, that was not a question to you, but a note to me.

>> > +static int show_only = 0;
>> > +static int remove_directories = 0;
>> > +static int quiet = 0;
>> > +static int ignored = 0;
>> > +static int ignored_only = 0;
>> 
>> Please do not explicitly initialize static variables to zero.
>
> I realize that static variables will be automatically initialized to
> zero so this is unnecessary, but is there some technical reason not to
> initialize explicitly?  If the answer is simply a style preference that
> is fine, I'm just here to learn.

Both readability and style have to do much with this.

The style has a historical background which is a slight
technical merit.  It results in a smaller executable file, as C
compilers traditionally placed file-scope static variables that
are not explicitly initialized in the BSS section, instead of
explicitly storing N-bytes of zero as the the initial data in it
(although I do not see a reason for compilers not to do the same
for variables explicitly initialized to zero.  In fact, I think
modern gcc produces the same allocation with or without "= 0"
initialization).

> Of course as already pointed out these don't actually need to be static
> in the first place so I'll simply move them into cmd_clean().  This does
> lead me to another question though.  Now that Dscho has converted my
> patch to use parse-options, what is the best way to update my patch
> while still giving credit to Dscho?

Please send a rewritten replacement version as a single patch
that is cleanly applicable to 'next', and mention people whose
input helped you in polishing the patch in the proposed commit
log message.