git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Matthieu Moy <Matthieu.Moy@imag.fr>, git <git@vger.kernel.org>
Subject: Re: [BUG] assertion failure in builtin-mv.c with "git mv -k"
Date: Thu, 15 Jan 2009 11:53:40 +0100	[thread overview]
Message-ID: <496F15B4.2040104@drmicha.warpmail.net> (raw)
In-Reply-To: <7vbpu91zjf.fsf@gitster.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 14.01.2009 20:02:
...
> If "git mv" did not have adequate test coverage, then please add a test
> script with both expect_success (for cases that should have been there
> when somebody worked on "git mv" originally), and expect_failure to expose
> the bug you found in your first patch.  Again, the second patch would
> update the code and flip expect_failure to expect_success.
> 
> I see there is t7001-mv and even though it claims to concentrate on its
> operation from subdirectory, it has tests for more basic modes of the
> operation.
> 
> So my recommendation would be to have a single patch that:
> 
>  (1) retitles t7001;
>  (2) adds your new -k tests to it; and
>  (3) adds the 1-liner fix.
> 

Sorry to bother you again, even after your detailed reply, but I'm a bit
confused in multiple ways (I guess that makes for multiple bits...).
First, you replied to my post, not my patch v2, but (time-wise) after my
patch v2, so I'm not sure whether your reply applies to v2 as well or
that one is OK.

Second, I took the title of t7001 to mean "mv into subdir" or "mv in and
out subdir" in order to distiguish it from "mv oldname newname", albeit
in English that leaves room from improvement.

Third, various parts of that test are in vastly different styles, and I
haven't found any test writing directions other than "follow what is
there", which leaves several alternatives. (Some use the test-lib repo,
some create their own underneath, some make assumptions about the
contents of "$TEST_DIRECTORY"/../.)

Fourth, t7001-mv is missing any test for "mv -k", and 2 of my 3
additional tests cover cases which work (pass) without the fix, which is
why I kept it separate, being unrelated. Following all your arguments
lead to the conclusion I should squash 2+3 (fix + mark expect_pass) -
until I read your conclusion, which says squash all.

I'm happy to follow any variant ("1+2+3", "1 2+3", "1 2 3", in
increasing order of preference) so there's no need to discuss or explain
this further, just tell me "do x" ;)

Cheers,
Michael

  reply	other threads:[~2009-01-15 10:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-14 13:20 [BUG] assertion failure in builtin-mv.c with "git mv -k" Matthieu Moy
2009-01-14 14:53 ` Michael J Gruber
2009-01-14 15:54   ` Johannes Schindelin
2009-01-14 16:04     ` Michael J Gruber
2009-01-14 16:47       ` Jeff King
2009-01-14 19:02       ` Junio C Hamano
2009-01-15 10:53         ` Michael J Gruber [this message]
2009-01-15 22:19           ` Junio C Hamano
2009-01-16 12:57             ` Matthieu Moy
2009-01-14 17:03     ` [PATCH v2 0/3] Add test cases for "git mv -k" and fix a known breakage Michael J Gruber
2009-01-14 17:03       ` [PATCH v2 1/3] add test cases for "git mv -k" Michael J Gruber
2009-01-14 17:03         ` [PATCH v2 2/3] fix handling of multiple untracked files for git mv -k Michael J Gruber
2009-01-14 17:03           ` [PATCH v2 3/3] mark fixed breakage as expect pass for "git mv -k" multiple files Michael J Gruber
2009-01-14 15:42 ` [PATCH] fix handling of multiple untracked files for git mv -k Michael J Gruber

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=496F15B4.2040104@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=Matthieu.Moy@imag.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).