Am 03.02.2014 23:56, schrieb Junio C Hamano:
> Jens Lehmann <[email protected]> writes:
>
>> + set_config_update_recurse_submodules(
>> +
>> parse_update_recurse_submodules_arg("--recurse-submodules-default",
>> + recurse_submodules_default),
>> + recurse_submodules);
>
> I think I saw these exact lines in another patch. Perhaps the whole
> thing can become a helper function that lets the caller avoid typing
> the whole long strings that needs a strange/unfortunate line break?
Right, that'd be better.
>> diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
>> index 06b18f8..bc3e1ca 100755
>> --- a/t/t2013-checkout-submodule.sh
>> +++ b/t/t2013-checkout-submodule.sh
>> @@ -4,17 +4,57 @@ test_description='checkout can handle submodules'
>>
>> . ./test-lib.sh
>>
>> +submodule_creation_must_succeed() {
>
> Style: SP before (), i.e.
>
> submodule_creation_must_succeed () {
>
>> + # checkout base ($1)
>> + git checkout -f --recurse-submodules $1 &&
>> + git diff-files --quiet &&
>> + git diff-index --quiet --cached $1 &&
>
> Please make it a habit to quote a parameter that is intended not to
> be split at $IFS (e.g. write these as "$1" not as $1). Otherwise
> the reader has to wonder if this can be called with a "foo bar" and
> the expects it to be split into two.
>
>> + # checkout target ($2)
>> + if test -d submodule; then
>
> Style: no semicolons in standard control structure, i.e.
>
> if test -d submodule
> then
>
>> + echo change>>submodule/first.t &&
>
> Style: SP before but not after redirection operator, i.e.
>
> echo foo >>bar
>
>> +submodule_removal_must_succeed() {
>
> Likewise.
>
>> + # checkout base ($1)
>> + git checkout -f --recurse-submodules $1 &&
>
> Likewise.
>
>> + echo first > file &&
>
> Likewise.
>
>> +test_expect_success '"checkout --recurse-submodules" replaces submodule
>> with files' '
>> + git checkout -f base &&
>> + git checkout -b replace_submodule_with_dir &&
>> + git update-index --force-remove submodule &&
>> + rm -rf submodule/.git .gitmodules &&
>> + git add .gitmodules submodule/* &&
>> + git commit -m "submodule replaced" &&
>> + git checkout -f base &&
>> + git submodule update -f &&
>> + git checkout --recurse-submodules replace_submodule_with_dir &&
>> + test -d submodule &&
>> + ! test -e submodule/.git &&
>> + test -f submodule/first.t &&
>> + test -f submodule/second.t
>> +'
>
> Hmmmm. Is it sufficient for these files to just exist, or do we
> want to make sure they have expected contents?
Thanks, will consider all you remarks above in the ongoing work for
testing framework which should replace these tests.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
