Re: [PATCH v2 4/4] fast-export: make sure refs are updated properly

[Felipe Contreras <felipe.contreras@gmail.com>]

On Wed, Oct 31, 2012 at 1:11 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> (cc-ing the git list)
> Felipe Contreras wrote:
>
>> When an object has already been exported (and thus is in the marks) it
>> is flagged as SHOWN, so it will not be exported again, even if this time
>> it's exported through a different ref.
>>
>> We don't need the object to be exported again, but we want the ref
>> updated
>
> Yes, makes perfect sense.
>
> For what it's worth,
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>

Yay!

> [...]
>> --- a/t/t5800-remote-helpers.sh
>> +++ b/t/t5800-remote-helpers.sh
>> @@ -145,4 +145,15 @@ test_expect_failure 'push new branch with old:new refspec' '
>>       compare_refs clone HEAD server refs/heads/new-refspec
>>  '
>>
>> +test_expect_success 'push ref with existing object' '
>> +     (cd localclone &&
>> +     git branch point-to-master master &&
>> +     git push origin point-to-master
>> +     ) &&
>> +
>> +     (cd server &&
>> +     git show-ref refs/heads/point-to-master
>> +     )
>
> Style: if you indent like this, the test becomes clearer:

And then it would become inconsistent with the rest of the file.

>> +     git fast-export --import-marks=tmp-marks \
>> +             --export-marks=tmp-marks master > actual &&
>> +     test_cmp expected actual
>
> Redirections in git shell scripts are generally spelled as
> "do_something >actual", without a space between the operator and
> filename.

I generally am OK with adapting to whatever code-style is used
(sometimes under protest), but this is a place where I draw   the
line. Sorry, '>actual' is more annoying to me than a knife screeching
glass. Fortunately, '> actual' is used in many other places in 't/',
so I'm going to use the other people jumping over the bridge argument.

Cheers.

-- 
Felipe Contreras