git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Paul Tan <pyokagan@gmail.com>,
	git@vger.kernel.org, Stefan Beller <sbeller@google.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 0/7] Improve git-pull test coverage
Date: Mon, 04 May 2015 10:35:55 -0700	[thread overview]
Message-ID: <xmqqlhh4tfd0.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <vpqoam0sqp5.fsf@anie.imag.fr> (Matthieu Moy's message of "Mon, 04 May 2015 10:16:22 +0200")

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Paul Tan <pyokagan@gmail.com> writes:
>
>> Paul Tan (7):
>>   t5520: test pulling multiple branches into an empty repository
>>   t5520: implement tests for no merge candidates cases
>>   t5520: test for failure if index has unresolved entries
>>   t5520: test work tree fast-forward when fetch updates head
>>   t5520: test --rebase with multiple branches
>>   t5520: test --rebase failure on unborn branch with index
>>   t5521: test --dry-run does not make any changes
>
> I did a semi-thourough review of the whole series, it looks good to me.

Thanks. I did the same, and it looked OK.

One test was duplicated with jc/merge series and caused a textual
conflict, but that was nothing major.

I didn't like the grepping of error messages alone as a way to
diagnose the failure, though.  When we expect the pull to fail
without touching anything in the working tree, I'd prefer to see
that tested explicitly (e.g. "if pull mistakenly tried to go ahead
and touch this file that would be involved in the merge, its
contents would change---let's make sure that does not happen by
making sure the contents before and after are the same" kind of
thing).

  reply	other threads:[~2015-05-04 17:36 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-02 15:37 [PATCH 0/7] Improve git-pull test coverage Paul Tan
2015-05-02 15:37 ` [PATCH 1/7] t5520: test pulling multiple branches into an empty repository Paul Tan
2015-05-02 15:37 ` [PATCH 2/7] t5520: implement tests for no merge candidates cases Paul Tan
2015-05-04  8:04   ` Matthieu Moy
2015-05-06  6:04     ` Paul Tan
2015-05-06  6:06       ` Paul Tan
2015-05-02 15:37 ` [PATCH 3/7] t5520: test for failure if index has unresolved entries Paul Tan
2015-05-04  8:09   ` Matthieu Moy
2015-05-02 15:37 ` [PATCH 4/7] t5520: test work tree fast-forward when fetch updates head Paul Tan
2015-05-03  2:42   ` Eric Sunshine
2015-05-03  2:47     ` Paul Tan
2015-05-02 15:37 ` [PATCH 5/7] t5520: test --rebase with multiple branches Paul Tan
2015-05-04 17:09   ` Stefan Beller
2015-05-04 19:24     ` Junio C Hamano
2015-05-05 16:00     ` Paul Tan
2015-05-02 15:37 ` [PATCH 6/7] t5520: test --rebase failure on unborn branch with index Paul Tan
2015-05-02 15:37 ` [PATCH 7/7] t5521: test --dry-run does not make any changes Paul Tan
2015-05-04  8:16 ` [PATCH 0/7] Improve git-pull test coverage Matthieu Moy
2015-05-04 17:35   ` Junio C Hamano [this message]
2015-05-05 10:39     ` Paul Tan
2015-05-06  6:30     ` Paul Tan

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=xmqqlhh4tfd0.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=pyokagan@gmail.com \
    --cc=sbeller@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).