Re: [PATCHv3 02/11] git-p4: test debug macro

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

 



gitster@xxxxxxxxx wrote on Sun, 18 Dec 2011 13:48 -0800:
> You may have found this handy yourself, but I would rather not to see it
> here in the current form for multiple reasons.
> 
>  - Why "ctrl-c"? You are not even spawning shell from here but are having
>    the user interact with this state in the middle of a test from another
>    shell, no?
> 
>    Why not "When done, type <RET>" and have a "read junk" or something
>    instead? That would be a lot simpler and you do not have to worry about
>    portability with many lines of comments.
> 
>    An alternative is to spawn an interactive shell here, and change the
>    "Debug me" comment to say "ctrl-d when done".

Good point.  I'll get rid of all that trap business.

>  - This is not linked to the generic "debug" option "txxxx-name.sh -d".
>    Shouldn't you be extending test_debug so that it can go interactive,
>    give customized comments&insns (i.e. "cd $here" may be useful for test
>    scripts outside testing p4, but "P4PORT=..." would not be, so the user
>    of test_debug in t9800-git-p4.* needs customizability of the message).

That's good advice; each (set of) script(s) could customize as
needed.

The pain of having to track down the P4PORT, which varies with
each test script, was my main motivation for trying to automate
this.

I'll not resubmit this one until coming up with something
prettier.

> Also could we please rename p4 related tests in t/t98* series so that it
> is clear that they are about git-p4 from "ls t/" output (i.e. have them
> all have "git-p4" in their names)?

Sure.  That was a mistake.  We'll fix it up.

		-- Pete
--
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]