Re: [PATCH 4/4] push: add 'prune' option

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

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Yeah, but that's 'git push --all', and that's not the common
> operation--'git push' is.

"git push" is common but that does not give you a solid base to guess what
the reader's assumption would be.  Are you assuming "matching" semantics?

> So that's what I presumed the reader would
> assume,...

I do not want let us guess what the reader assumes, as many people seem to
suggest setting push.default to different values and that would change
what the reader would assume.  That was the whole reason that I suggested
to spell the assumption out, so that the reader's assumption does not have
to get into the picture.

> This reuses the name `tmp`, which seems to be your objective, but it
> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is
> the upstream branch, or is it because it has the same name?

The example is to clarify "local counterpart" in the main text.  I
actually would prefer to get rid of `tmp` but I left it as-is as you
wrote.  The exact name used in the example does not matter, whether it is
`tmp` or `xyzzy`.

> Unfortunately, I as a reader have trouble understanding this. More
> specifically I have trouble understanding where `refs/remotes/laptop/`
> is coming from, and what it is meaning. I have always pictured
> `refs/remotes` as something that 'git fetch' updates, and always from
> the relevant repository.

The layout is the recommended set-up to emulate a fetch with a push in the
reverse direction, which I thought anybody should notice.  It is a failure
in our documentation that even an expert didn't.

>>> diff --git a/remote.h b/remote.h
>>> index b395598..341142c 100644
>>> --- a/remote.h
>>> +++ b/remote.h
>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);
>>>  enum match_refs_flags {
>>>       MATCH_REFS_NONE         = 0,
>>>       MATCH_REFS_ALL          = (1 << 0),
>>> -     MATCH_REFS_MIRROR       = (1 << 1)
>>> +     MATCH_REFS_MIRROR       = (1 << 1),
>>> +     MATCH_REFS_PRUNE        = (1 << 2),
>>>  };
>>
>> Lose the ',' at the end, for the same reason why deleted line did not have
>> one.
>
> And why is that?

Because I told you so ;-).

More seriously, we have had patches to accomodate other people's compilers
by dropping the last comma in enum {}.  See c9b6782 (enums: omit trailing
comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma
for portability, 2010-05-14) for examples.