public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Lex Spoon <lex@lexspoon•org>
Cc: Luke Diamand <luke@diamand•org>, Git Users <git@vger•kernel.org>,
	Pete Wyckoff <pw@padd•com>
Subject: Re: [PATCH] git-p4: Use -m when running p4 changes
Date: Thu, 16 Apr 2015 11:56:31 -0700	[thread overview]
Message-ID: <xmqqfv7zj40w.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CALM2SnafBHz8YeWtUtQDUgLBP_s9AiJy=9UC6XveqP0zrYMEqA@mail.gmail.com> (Lex Spoon's message of "Tue, 14 Apr 2015 23:47:40 -0400")

Lex Spoon <lex@lexspoon•org> writes:

> From 9cc607667a20317c837afd90d50c078da659b72f Mon Sep 17 00:00:00 2001
> From: Lex Spoon <lex@lexspoon•org>
> Date: Sat, 11 Apr 2015 10:01:15 -0400
> Subject: [PATCH] git-p4: Use -m when running p4 changes

All of the above is duplicate and shouldn't be added to the message;
the recipient can pick them up from the e-mail headers.

Please explain what this change intends to do (e.g. Is it a fix?  If
so, what is broken without this change?  Is it an enhancement?  If
so, what cannot be done without this change, and how and why is the
new thing the change enables a good thing?), and why it is a good
idea to use "-m" to realize that objective.

> Signed-off-by: Lex Spoon <lex@lexspoon•org>
> ---
> Updated to include a test case
>
>  git-p4.py               | 51 ++++++++++++++++++++++++++++++---------
>  t/t9818-git-p4-block.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 104 insertions(+), 11 deletions(-)
>  create mode 100755 t/t9818-git-p4-block.sh
>
> diff --git a/git-p4.py b/git-p4.py
> index 549022e..2fc8d9c 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -740,17 +740,43 @@ def
> createOrUpdateBranchesFromOrigin(localRefPrefix = "refs/remotes/p4/",
> silent
>  def originP4BranchesExist():
>          return gitBranchExists("origin") or
> gitBranchExists("origin/p4") or gitBranchExists("origin/p4/master")

It appears that the patch is severely linewrapped.

> diff --git a/t/t9818-git-p4-block.sh b/t/t9818-git-p4-block.sh
> new file mode 100755
> index 0000000..73e545d
> --- /dev/null
> +++ b/t/t9818-git-p4-block.sh
> @@ -0,0 +1,64 @@
> +#!/bin/sh
> +
> +test_description='git p4 fetching changes in multiple blocks'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> + start_p4d
> +'

We do not do one-SP indent.  Indent with tab instead.

> +
> +test_expect_success 'Create a repo with 100 changes' '
> + (
> + cd "$cli" &&
> + touch file.txt &&

Do not use "touch" when the only thing you are interested in is that
the file exists and you do not care about its timestamp.  I.e. say

    >file.txt &&

instead.

> + p4 add file.txt &&
> + p4 submit -d "Add file.txt" &&
> + for i in 0 1 2 3 4 5 6 7 8 9
> + do
> + touch outer$i.txt &&
> + p4 add outer$i.txt &&
> + p4 submit -d "Adding outer$i.txt" &&
> + for j in 0 1 2 3 4 5 6 7 8 9
> + do
> + p4 edit file.txt &&
> + echo $i$j > file.txt &&
> + p4 submit -d "Commit $i$j"
> + done
> + done
> + )

What happens when any of these commands in the &&-chain fails?

	(
        	cd "$cli" &&
                >file.txt &&
		p4 ... &&
                for i in $(test_seq ...)
                do
                	>"outer$i.txt" &&
                        p4 ... &&
			for j in $(test_seq ...)
			do
				p4 ... &&
				p4 ... || exit
			done
		done
	)

or something like that, perhaps?

  reply	other threads:[~2015-04-16 18:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CALM2SnY62u3OXJOMSqSfghH_NYwZhzSedm3-wcde-dQCX6eB9Q@mail.gmail.com>
2015-04-14 11:25 ` [PATCH] git-p4: Use -m when running p4 changes Luke Diamand
     [not found]   ` <CALM2SnY=ZcSMSXk6Ks0uU65gPX5vC8QKG+iSrQxd3X7N=sw+Ww@mail.gmail.com>
2015-04-14 19:48     ` Lex Spoon
2015-04-15  3:47   ` Lex Spoon
2015-04-16 18:56     ` Junio C Hamano [this message]
2015-04-16 23:15     ` Luke Diamand
2015-04-17 13:20       ` Lex Spoon
2015-04-17 23:11         ` [PATCH v3] " Lex Spoon
2015-04-20  9:53           ` Luke Diamand
2015-04-20 14:30             ` Lex Spoon
2015-04-20 15:00               ` [PATCH v4] " Lex Spoon
2015-04-20 15:15                 ` Luke Diamand
2015-04-20 15:25                   ` Lex Spoon
2015-04-20 19:17                     ` Luke Diamand
2015-04-20 17:54                   ` Junio C Hamano
2015-04-20 18:04                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfv7zj40w.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=lex@lexspoon$(echo .)org \
    --cc=luke@diamand$(echo .)org \
    --cc=pw@padd$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox