git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [RFC PATCH 1/5] Add testcases for improved file collision conflict handling
Date: Thu, 8 Mar 2018 09:51:56 -0800	[thread overview]
Message-ID: <CABPp-BGO1m=OAbUS8RUKdqP+8p-DokzbjGa3TcigmTSSa_r3-Q@mail.gmail.com> (raw)
In-Reply-To: <20180308122523.14434-1-szeder.dev@gmail.com>

Sweet!  Thanks for taking a look, and for spotting lots of
improvements (and some really embarrassing errors).  I'll keep the
fixes queued up while waiting for other feedback.  A few comments...

On Thu, Mar 8, 2018 at 4:25 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:

> This setup test is enormous, and the conditions for the combination of
> the two sides and the add/rename conflicts are somewhat distracting.
> I don't know how it could be structured better/shorter/clearer...  I
> couldn't come up with anything useful during lunch.

Yeah.  :-(  It's part attempt to test these conflict types much more
thoroughly than other tests in the testsuite do, and part attempt to
keep the test setup consistent between the types to reflect the fact
that I'm consolidating the conflict resolution into a single function
as well.

Two possible ideas:

  * Split the tests for "*_unrelated" files and "*_related" files into
separate tests (doubling the number of tests, but making each only
deal with half the files.  That would make each setup function about
half the size, though both check functions would be about as big as
the original.

  * Instead of programatically generated tests, just manually write
out the tests for each of the four combinations (rename/rename,
rename/add, add/rename, add/add).  That means four "copies" of fairly
similar functions (and possible greater difficulty keeping things
consistent if changes are requested), but does allow removal of the
three different if-statements and thus makes each one easier to
understand in isolation.

Doing both would be possible as well.

Personally, I'm much more in favor of the first idea than the second.
I'm still kind of borderline about whether to make the change
mentioned in the first idea, but if others feel that splitting would
help a lot, I'm happy to look into either or both ideas.

>> +                     cat <<EOF >>expected &&
>> +<<<<<<< HEAD
>> +modification
>> +=======
>> +more stuff
>> +yet more stuff
>> +>>>>>>> R^0
>> +EOF
>
> You could use 'cat <<-EOF ....' and indent the here doc with tabs, so
> it won't look so out-of-place.  Or even '<<-\EOF' to indicate that
> there is nothing to be expanded in the here doc.

I just learned two new things about heredocs; I've wanted both of
those things in the past, but didn't even think to check if they were
possible.  Thanks for enlightening me.

  reply	other threads:[~2018-03-08 17:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-05 17:11 [RFC PATCH 0/5] Improve path collision conflict resolutions Elijah Newren
2018-03-05 17:11 ` [RFC PATCH 1/5] Add testcases for improved file collision conflict handling Elijah Newren
2018-03-08 12:25   ` SZEDER Gábor
2018-03-08 17:51     ` Elijah Newren [this message]
2018-03-05 17:11 ` [RFC PATCH 2/5] merge-recursive: new function for better colliding conflict resolutions Elijah Newren
2018-03-05 17:11 ` [RFC PATCH 3/5] merge-recursive: fix rename/add conflict handling Elijah Newren
2018-03-05 17:11 ` [RFC PATCH 4/5] merge-recursive: improve handling for rename/rename(2to1) conflicts Elijah Newren
2018-03-05 17:11 ` [RFC PATCH 5/5] merge-recursive: improve handling for add/add conflicts Elijah Newren
2018-03-12 18:19 ` [RFC PATCH 0/5] Improve path collision conflict resolutions Elijah Newren

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CABPp-BGO1m=OAbUS8RUKdqP+8p-DokzbjGa3TcigmTSSa_r3-Q@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=szeder.dev@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).