From: Elijah Newren <newren@gmail.com> To: Junio C Hamano <gitster@pobox.com> Cc: "Git Mailing List" <git@vger.kernel.org>, corrmage@gmail.com, "Ævar Arnfjörð" <avarab@gmail.com>, "Johannes Schindelin" <Johannes.Schindelin@gmx.de>, "Stefan Beller" <sbeller@google.com> Subject: Re: [PATCH 1/3] t3401: add another directory rename testcase for rebase and am Date: Thu, 30 Aug 2018 09:26:59 -0700 Message-ID: <CABPp-BErSY+284vqD6xiLrcsbMOghzM7vSp6THXkYuCbymUqKA@mail.gmail.com> (raw) In-Reply-To: <xmqqlg8n6dy4.fsf@gitster-ct.c.googlers.com> On Thu, Aug 30, 2018 at 9:01 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > >> Is this a single-shot environment assignment? That would have been > >> caught with the test linter. > > > > Ugh, yes. Sorry. > > > > I was trying to allow backporting to 2.18, so tried to build my series > > on that...but moved forward slightly to en/rebase-consistency in order > > to re-use the same testfile (as mentioned in my cover letter). But > > that meant my branch was missing a0a630192dca > > ("t/check-non-portable-shell: detect "FOO=bar shell_func"", > > 2018-07-13), so the test-linter couldn't catch this for me. > > True, I also only caught this during my integration cycle, not > during the review of the patches. > > >> Perhaps squashing this in would be sufficient fix? > > > > Thanks for fixing it up. That works...although now I've spotted > > another minor issue. The FAKE_LINES setting is only needed for the > > interactive rebase case and can be removed for the other two. Oops. > > It doesn't hurt, but might confuse readers of the testcase. > > Ah, OK. So the squashable fix would now become like this, all > fixing the ones from the first patch. > > > Would you like me to resend a fixup on top of your fixup, resend the > > whole series with the fixes squashed, or just ignore this final > > (cosmetic) issue since it doesn't affect correctness and I've caused > > you enough extra work already? > > No worries. This is what the maintainer does (when s/he is not > playing other roles like a reviewer or an individual contributor). > > I'll squash in the following to the 1/3 patch and queue the topic > with the other two patches. Thanks, looks great. > diff --git a/t/t3401-rebase-and-am-rename.sh b/t/t3401-rebase-and-am-rename.sh > index 94bdfbd69c..e0b5111993 100755 > --- a/t/t3401-rebase-and-am-rename.sh > +++ b/t/t3401-rebase-and-am-rename.sh > @@ -141,7 +141,7 @@ test_expect_success 'rebase --interactive: NO directory rename' ' > git checkout B^0 && > > set_fake_editor && > - FAKE_LINES="1" test_must_fail git rebase --interactive A && > + test_must_fail env FAKE_LINES="1" git rebase --interactive A && > > git ls-files -s >out && > test_line_count = 6 out && > @@ -160,7 +160,7 @@ test_expect_success 'rebase (am): NO directory rename' ' > git checkout B^0 && > > set_fake_editor && > - FAKE_LINES="1" test_must_fail git rebase A && > + test_must_fail git rebase A && > > git ls-files -s >out && > test_line_count = 6 out && > @@ -179,7 +179,7 @@ test_expect_success 'rebase --merge: NO directory rename' ' > git checkout B^0 && > > set_fake_editor && > - FAKE_LINES="1" test_must_fail git rebase --merge A && > + test_must_fail git rebase --merge A && > > git ls-files -s >out && > test_line_count = 6 out &&
next prev parent reply other threads:[~2018-08-30 16:27 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-28 12:27 A rebase regression in Git 2.18.0 Nikolay Kasyanov 2018-08-28 13:17 ` Ævar Arnfjörð Bjarmason 2018-08-28 13:33 ` Johannes Schindelin 2018-08-28 13:46 ` Nikolay Kasyanov 2018-08-28 15:35 ` Elijah Newren 2018-08-28 16:58 ` Junio C Hamano 2018-08-29 7:06 ` [PATCH 0/3] Turn off directory rename detection in am -3 Elijah Newren 2018-08-29 7:06 ` [PATCH 1/3] t3401: add another directory rename testcase for rebase and am Elijah Newren 2018-08-29 22:12 ` Junio C Hamano 2018-08-29 23:47 ` Elijah Newren 2018-08-30 16:01 ` Junio C Hamano 2018-08-30 16:26 ` Elijah Newren [this message] 2018-08-29 7:06 ` [PATCH 2/3] merge-recursive: add ability to turn off directory rename detection Elijah Newren 2018-08-29 12:54 ` Johannes Schindelin 2018-08-29 23:00 ` Elijah Newren 2018-08-29 7:06 ` [PATCH 3/3] am: avoid directory rename detection when calling recursive merge machinery Elijah Newren 2018-08-29 12:51 ` Johannes Schindelin 2018-08-30 16:41 ` A rebase regression in Git 2.18.0 Elijah Newren 2018-08-31 10:11 ` Johannes Schindelin 2018-08-31 19:37 ` 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-BErSY+284vqD6xiLrcsbMOghzM7vSp6THXkYuCbymUqKA@mail.gmail.com \ --to=newren@gmail.com \ --cc=Johannes.Schindelin@gmx.de \ --cc=avarab@gmail.com \ --cc=corrmage@gmail.com \ --cc=git@vger.kernel.org \ --cc=gitster@pobox.com \ --cc=sbeller@google.com \ --subject='Re: [PATCH 1/3] t3401: add another directory rename testcase for rebase and am' \ /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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: 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 # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. 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.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git