On Thu, Apr 25, 2013 at 3:06 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Felipe Contreras <felipe.contre...@gmail.com> writes:
>> On Thu, Apr 25, 2013 at 1:25 PM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Felipe Contreras <felipe.contre...@gmail.com> writes:
>>>>>> ...
>>>>>> +             git for-each-ref --format='%(refname) %(objectname)' |
>>>>>> +             while read ref a
>>>>>>               do
>>>>>> -                     test $a == $b && continue
>>>>>> +                     case "$before" in
>>>>>> +                     *"$ref $a"*)
>>>>>> +                             continue
>>>> I wonder if we should bother with this at all. The purpose of the code
>>>> was mainly to show to users that they should report the success only
>>>> if the refs have been updated, but the code is becoming more
>>>> obfuscated, a comment should do the trick. And then, we can just
>>>> report success for all the refs (and explain in the comment why).
>>> Are you proposing to say "ok $ref" to everything we see in the
>>> resulting repository, even the ones the caller of remote-testgit did
>>> not ask us to do anything with?
>>> Wouldn't the caller be surprised if we did so?
>> Why would it?  The only effective difference is what you'll see
>> reported in the UI, but there's no user here.
> You are correct that it affects only the UI, but isn't that because
> the current implementation of push_update_refs_status() blindly
> accepts whatever 'ok' response says without checking the ref
> mentioned against what it tried to push out?  I was wondering if
> that codepath should stay that way forever, or if we may want add
> sanity checks later.  If the latter, I suspect this would manifest
> as an ancient bug to say 'ok' for everything we have instead of what
> we actually pushed out (of course, the explanation in the comment
> would help).

Actually, I think the code already checks for that.

> So I am OK with that simpler approach.  Care to roll the conclusion
> of your idea into a patch?

Will do.

> By the way, I noticed that Documentation/gitremote-helpers.txt does
> not even mention these 'ok' responses for 'export' command, but they
> should be the same as responses to 'push'. Perhaps we can share some
> text between the two?

Indeed, it's the same code for both.

Felipe Contreras
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

Reply via email to