From: Elijah Newren <firstname.lastname@example.org> To: SZEDER Gábor <email@example.com> Cc: Git Mailing List <firstname.lastname@example.org> Subject: Re: [RFC PATCH 1/5] Add testcases for improved file collision conflict handling Date: Thu, 8 Mar 2018 09:51:56 -0800 Message-ID: <CABPp-BGO1m=OAbUS8RUKdqP+8p-DokzbjGa3TcigmTSSa_r3-Q@mail.gmail.com> (raw) In-Reply-To: <email@example.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 <firstname.lastname@example.org> 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.
next prev parent reply index 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 publically 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' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.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
firstname.lastname@example.org mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox