Re: [PATCHv4 3/3] Advices about 'git rm' during conflicts (unmerged paths) more relevant

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kong Lucien <Lucien.Kong@xxxxxxxxxxxxxxx> writes:

> Subject: Re: [PATCHv4 3/3] Advices about 'git rm' during conflicts (unmerged paths) more relevant

"more relevant" doesn't say much, and your subject line doesn't say it's
about "status". What about

status: don't suggest "git rm" if not appropriate

?

> +test_expect_success 'status when conflicts without rm advice (both modified)' '

How is this different from the other existing tests? Avoid adding
redundant tests, they slow down the test suite without added value.

> +test_expect_success 'status when conflicts with add and rm advice (deleted by us)' '
> +test_expect_success 'status when conflicts with add and rm advice (deleted by them)' '

You can probably save a few line (and test execution time) by reusing
the same repository in the second test, like

git reset --hard
git checkout master
git merge second_branch

(i.e. reply the merge, but symmetrically, from the other branch)

Perhaps this sould be added to t7060-wtstatus.sh (basic work tree status
reporting) instead, which already tests "deleted by us:". Ideally, you
could add tests for "deleted by them" and "both deleted" in a first
patch, and turn the add/rm into add in the implementation patch.

+	# Unmerged paths:
+	#   (use "git add/rm <file>..." as appropriate to mark resolution)
+	#
+	#	deleted by us:      main.txt
+	#

To really test the logic, you need to have more than one file in the
list (the logic should say "suggest rm if _at least one_ file may need
it", but currently, a patch implementing "if _all_ files" would pass the
test).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]