Re: [PATCH v2 3/3] git-p4: Add test case for complex branch import

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 23, 2012 at 1:53 PM, Vitor Antunes <vitor.hda@xxxxxxxxx> wrote:
> On Sat, Jan 21, 2012 at 4:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Vitor Antunes <vitor.hda@xxxxxxxxx> writes:
>>
>>> +test_expect_success 'git-p4 add complex branches' '
>>> +     test_when_finished cleanup_git &&
>>> +     test_create_repo "$git" &&
>>> +     (
>>> +             cd "$cli" &&
>>> +             changelist=$(p4 changes -m1 //depot/... | cut -d" " -f2) &&
>>> +             changelist=$(($changelist - 5)) &&
>>> +             p4 integrate //depot/branch1/...@$changelist //depot/branch4/... &&
>>> +             p4 submit -d "branch4" &&
>>> +             changelist=$(($changelist + 2)) &&
>>> +             p4 integrate //depot/branch1/...@$changelist //depot/branch5/... &&
>>> +             p4 submit -d "branch5"
>>
>> That's a strange quoting convention. Why are "branch4" and "branch5"
>> enclosed in double quotes while "integrate" and "submit" aren't?
>> (rhetorical: do not quote these branch names without a good reason).
>
> There is no reason that I can remember to have those enclosed in double
> quotes. Will double check in my local branches at home tonight. Anyway,
> expect a fix for this in v3.

I now see why I added the quotes. The -d option is used to input the
description of the commit, which can contain spaces and other special
characters. Admittedly they are not required in this case, but from a
consistency point of view I would prefer to keep them. Is this
acceptable?

>>> +# Configure branches through git-config and clone them. git-p4 will only be able
>>> +# to clone the original structure if it is able to detect the origin changelist
>>> +# of each branch.
>>> +test_expect_success 'git-p4 clone complex branches' '
>>> +     test_when_finished cleanup_git &&
>>> +     test_create_repo "$git" &&
>>> +     (
>>> +             cd "$git" &&
>>> +             git config git-p4.branchList branch1:branch2 &&
>>> +             git config --add git-p4.branchList branch1:branch3 &&
>>> +             git config --add git-p4.branchList branch1:branch4 &&
>>> +             git config --add git-p4.branchList branch1:branch5 &&
>>> +             "$GITP4" clone --dest=. --detect-branches //depot@all &&
>>> +             git log --all --graph --decorate --stat &&
>>> +             git reset --hard p4/depot/branch1 &&
>>> +             test_path_is_file file1 &&
>>> +             test_path_is_file file2 &&
>>> +             test_path_is_file file3 &&
>>> +             grep -q update file2 &&
>>
>> Do you really need to use "-q" here?  Wouldn't it help if you wrote it
>> without it while debugging tests with "sh ./t9801-*.sh -v"?
>
> Makes sense.
>
> Thank you for the helpful comments.
> Vitor

Apparently I've hit "Reply" instead of "Reply all" before, so I'll keep
the full quote of my previous email for future reference.

Vitor
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]