All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Nikola Forró" <nforro@redhat.com>, git@vger.kernel.org
Subject: Re: [PATCH v2] difftool/mergetool: make the form of yes/no questions consistent
Date: Tue, 12 Apr 2016 18:25:01 -0700	[thread overview]
Message-ID: <20160413012501.GA32611@gmail.com> (raw)
In-Reply-To: <xmqqegaapm02.fsf@gitster.mtv.corp.google.com>

On Tue, Apr 12, 2016 at 02:15:09PM -0700, Junio C Hamano wrote:
> Nikola Forró <nforro@redhat.com> writes:
> 
> > Every yes/no question in difftool/mergetool scripts has slightly
> > different form, and none of them is consistent with the form git
> > itself uses.
> >
> > Make the form of all the questions consistent with the form used
> > by git.
> >
> > Reviewed-by: John Keeping <john@keeping.me.uk>
> > Signed-off-by: Nikola Forró <nforro@redhat.com>
> > ---
> > Changes in v2: example dropped from the commit message
> 
> Thanks; have you run the test suite with this patch applied?
> 
> It is your responsibility to make sure that the expectation by
> existing tests are still satisfied after your change, or update
> their expectation to match the new (and hopefully better) world
> order your patch introduces.
> 
> I needed to squash this in to make the tests pass, but because I am
> not a difftool user, I do not at all know if the prompt produced
> (and expected by the test) is sensible or not.
> 
>  t/t7800-difftool.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
> index ec8bc8c..df9050f 100755
> --- a/t/t7800-difftool.sh
> +++ b/t/t7800-difftool.sh
> @@ -20,7 +20,7 @@ difftool_test_setup ()
>  prompt_given ()
>  {
>  	prompt="$1"
> -	test "$prompt" = "Launch 'test-tool' [Y/n]: branch"
> +	test "$prompt" = "Launch 'test-tool' [Y/n]? branch"
>  }
>  
>  # Create a file on master and change it on branch

That looks correct to me.  Sorry 'bout that, I'll remember to
run the tests with the patches applied next time.

cheers,
-- 
David

  reply	other threads:[~2016-04-13  1:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-12 14:44 [PATCH v2] difftool/mergetool: make the form of yes/no questions consistent Nikola Forró
2016-04-12 21:15 ` Junio C Hamano
2016-04-13  1:25   ` David Aguilar [this message]
2016-04-13 10:27   ` Nikola Forró

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=20160413012501.GA32611@gmail.com \
    --to=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nforro@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.