On Thu, Apr 25, 2013 at 3:06 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: > >> On Thu, Apr 25, 2013 at 1:25 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>> Felipe Contreras <felipe.contreras@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html