threads / patch / 41460

v2, 5 partsTests and fixes for merge-recursive rename options

Subject: [PATCH v2 0/5] Tests and fixes for merge-recursive rename options

## tl;dr

11 messages between Feb 21, 2016 and Feb 22, 2016. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · lore

This is a reorganisation of the previous series, bundling the test for the fix along with the commit itself, as suggested by Eric. It also includes many fixes and improvements pointed out by the same reviewer, whom I thank.

The typo fix is the same as before.

In "add rename threshold tests", I include tests involving --find-renames, except for one that depends on the fix.

"test option to disable renames" adds tests involving --rename-threshold.
"test deprecated interface" tests the aliasing --rename-threshold.

In "find-renames resets threshold", the specific test for the feature was bundled along.

To Junio: Please pay special attention to the test of threshold truncation. Given that it seems to be an undocumented feature, I am not sure whether it should be included or not.

Felipe Gonçalves Assis (5):
  merge-strategies.txt: fix typo
  t3034: add rename threshold tests
  t3034: test option to disable renames
  t3034: test deprecated interface
  merge-recursive: find-renames resets threshold
 Documentation/merge-strategies.txt                 |   4 +-
 merge-recursive.c                                  |   4 +-
 ...s.sh => t3032-merge-recursive-space-options.sh} |   2 +-
 t/t3034-merge-recursive-rename-options.sh          | 235 +++++++++++++++++++++
 4 files changed, 241 insertions(+), 4 deletions(-)
 rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
 create mode 100755 t/t3034-merge-recursive-rename-options.sh
-- 
2.7.1.492.gd821b20
Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v2 1/5] merge-strategies.txt: fix typo

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
 Documentation/merge-strategies.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Show changes to Documentation/merge-strategies.txt +2 −2
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index ff359b6..2eb92b9 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -86,8 +86,8 @@ no-renames;;
 	See also linkgit:git-diff[1] `--no-renames`.
 
 find-renames[=<n>];;
-	Turn on rename detection, optionally setting the the similarity
-	threshold. This is the default.
+	Turn on rename detection, optionally setting the similarity
+	threshold.  This is the default.
 	See also linkgit:git-diff[1] `--find-renames`.
 
 rename-threshold=<n>;;
-- 
2.7.1.492.gd821b20
Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v2 2/5] t3034: add rename threshold tests

10ae752 (merge-recursive: option to specify rename threshold, 2010-09-27) introduced this feature but did not include any tests.

The tests use the new option --find-renames, which replaces the then introduced and now deprecated option --rename-threshold.

Also update name and description of t3032 for consistency: "merge-recursive options" -> "merge-recursive space options"

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
Now this already introduces the final names and descriptions for the tests.

The test for threshold truncation was kept for now. Please confirm whether it is desirable.

New tests were added at the end to check that invalid arguments to find-renames= such as negative and non-numbers error out.

 ...s.sh => t3032-merge-recursive-space-options.sh} |   2 +-
 t/t3034-merge-recursive-rename-options.sh          | 159 +++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)
 rename t/{t3032-merge-recursive-options.sh => t3032-merge-recursive-space-options.sh} (99%)
 create mode 100755 t/t3034-merge-recursive-rename-options.sh
Show changes to 3 files +160 −1

t/t3032-merge-recursive-options.sh, t/t3032-merge-recursive-space-options.sh, t/t3034-merge-recursive-rename-options.sh

diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-space-options.sh
similarity index 99%
rename from t/t3032-merge-recursive-options.sh
rename to t/t3032-merge-recursive-space-options.sh
index 4029c9c..b56180e 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-space-options.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='merge-recursive options
+test_description='merge-recursive space options
 
 * [master] Clarify
  ! [remote] Remove cruft
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
new file mode 100755
index 0000000..7ae7f83
--- /dev/null
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -0,0 +1,159 @@
+#!/bin/sh
+
+test_description='merge-recursive rename options
+
+Test rename detection by examining rename/delete conflicts.
+
+Similarity index:
+R100 a-old a-new
+R075 b-old b-new
+R050 c-old c-new
+R025 d-old d-new
+'
+
+. ./test-lib.sh
+
+get_expected_stages () {
+	git checkout rename -- $1-new &&
+	git ls-files --stage $1-new >expected-stages-undetected-$1 &&
+	sed "s/ 0	/ 2	/" <expected-stages-undetected-$1 \
+		>expected-stages-detected-$1 &&
+	git read-tree -u --reset HEAD
+}
+
+rename_detected () {
+	git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+	test_cmp expected-stages-detected-$1 stages-actual-$1
+}
+
+rename_undetected () {
+	git ls-files --stage $1-old $1-new >stages-actual-$1 &&
+	test_cmp expected-stages-undetected-$1 stages-actual-$1
+}
+
+check_common () {
+	git ls-files --stage >stages-actual &&
+	test_line_count = 4 stages-actual
+}
+
+check_find_renames_25 () {
+	check_common &&
+	rename_detected a &&
+	rename_detected b &&
+	rename_detected c &&
+	rename_detected d
+}
+
+check_find_renames_50 () {
+	check_common &&
+	rename_detected a &&
+	rename_detected b &&
+	rename_detected c &&
+	rename_undetected d
+}
+
+check_find_renames_75 () {
+	check_common &&
+	rename_detected a &&
+	rename_detected b &&
+	rename_undetected c &&
+	rename_undetected d
+}
+
+check_find_renames_100 () {
+	check_common &&
+	rename_detected a &&
+	rename_undetected b &&
+	rename_undetected c &&
+	rename_undetected d
+}
+
+test_expect_success setup '
+	cat <<-\EOF >a-old &&
+	aa1
+	aa2
+	aa3
+	aa4
+	EOF
+	sed s/aa/bb/ <a-old >b-old &&
+	sed s/aa/cc/ <a-old >c-old &&
+	sed s/aa/dd/ <a-old >d-old &&
+	git add [a-d]-old &&
+	git commit -m base &&
+	git rm [a-d]-old &&
+	git commit -m delete &&
+	git checkout -b rename HEAD^ &&
+	cp a-old a-new &&
+	sed 1,1s/./x/ <b-old >b-new &&
+	sed 1,2s/./x/ <c-old >c-new &&
+	sed 1,3s/./x/ <d-old >d-new &&
+	git add [a-d]-new &&
+	git rm [a-d]-old &&
+	git commit -m rename &&
+	get_expected_stages a &&
+	get_expected_stages b &&
+	get_expected_stages c &&
+	get_expected_stages d
+'
+
+test_expect_success 'default similarity threshold is 50%' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'low rename threshold' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
+test_expect_success 'high rename threshold' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=75 HEAD^ -- HEAD master &&
+	check_find_renames_75
+'
+
+test_expect_success 'exact renames only' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=100% HEAD^ -- HEAD master &&
+	check_find_renames_100
+'
+
+test_expect_success 'rename threshold is truncated' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=200% HEAD^ -- HEAD master &&
+	check_find_renames_100
+'
+
+test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=25 --find-renames=75 HEAD^ -- HEAD master &&
+	check_find_renames_75
+'
+
+test_expect_success 'assumption for further tests: trivial merge succeeds' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive HEAD -- HEAD HEAD &&
+	git diff --quiet --cached &&
+	git merge-recursive --find-renames=25 HEAD -- HEAD HEAD &&
+	git diff --quiet --cached &&
+	git merge-recursive --find-renames=75 HEAD -- HEAD HEAD &&
+	git diff --quiet --cached &&
+	git merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&
+	git diff --quiet --cached
+'
+
+test_expect_success '--find-renames rejects negative argument' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=-25 HEAD -- HEAD HEAD &&
+	git diff --quiet --cached
+'
+
+test_expect_success '--find-renames rejects non-numbers' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=0xf HEAD -- HEAD HEAD &&
+	git diff --quiet --cached
+'
+
+test_done
-- 
2.7.1.492.gd821b20
Junio C Hamano· Feb 22, 2016, 19:03 UTC · re: Felipe Gonçalves Assis · lore

Re: [PATCH v2 2/5] t3034: add rename threshold tests

"Felipe Gonçalves Assis"  <felipeg.assis@gmail.com> writes:
Show 52 quoted lines
> 10ae752 (merge-recursive: option to specify rename threshold,
> 2010-09-27) introduced this feature but did not include any tests.
>
> The tests use the new option --find-renames, which replaces the then
> introduced and now deprecated option --rename-threshold.
> ...
> diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
> new file mode 100755
> index 0000000..7ae7f83
> --- /dev/null
> +++ b/t/t3034-merge-recursive-rename-options.sh
> @@ -0,0 +1,159 @@
> +#!/bin/sh
> +
> +test_description='merge-recursive rename options
> +
> +Test rename detection by examining rename/delete conflicts.
> +
> +Similarity index:
> +R100 a-old a-new
> +R075 b-old b-new
> +R050 c-old c-new
> +R025 d-old d-new
> +'
> ...
> +test_expect_success setup '
> +	cat <<-\EOF >a-old &&
> +	aa1
> +	aa2
> +	aa3
> +	aa4
> +	EOF
> +	sed s/aa/bb/ <a-old >b-old &&
> +	sed s/aa/cc/ <a-old >c-old &&
> +	sed s/aa/dd/ <a-old >d-old &&
> +	git add [a-d]-old &&
> +	git commit -m base &&
> +	git rm [a-d]-old &&
> +	git commit -m delete &&
> +	git checkout -b rename HEAD^ &&
> +	cp a-old a-new &&
> +	sed 1,1s/./x/ <b-old >b-new &&
> +	sed 1,2s/./x/ <c-old >c-new &&
> +	sed 1,3s/./x/ <d-old >d-new &&
> +	git add [a-d]-new &&
> +	git rm [a-d]-old &&
> +	git commit -m rename &&
> +	get_expected_stages a &&
> +	get_expected_stages b &&
> +	get_expected_stages c &&
> +	get_expected_stages d
> +'

I somehow doubt that it is wise to make the similarity index computed by the current heuristics as a hard promise like this test does.

This test specifies that turning the original bb1/bb2/bb3/bb4 into updated xb1/bb2/bb3/bb4 _MUST_ get 75% similarity, but that is not something we want to guarantee, ever. We may later update the algorithm and tweak such a change to register 70% or 78%, but such a change would break the expectation by this test. This test script however should not be interested in the exact similarity index assignment for a given filepair--it only wants to make sure that the option chooses the filepair with similarity index above the given value.

The test for --find-renames=<num> should instead only validate that the code works on the value given from the command line relative to the similarity index the code computed. I.e. first measure what the similarity index going from b-old to b-new is (e.g. it may say 76%, or 74%, depending on the version of Git being tested), then choose a value that is higher (or lower) than that similarity to give to the --find-renames=<num> option and ensure that merge-recursive does what is expected.

I am not very happy with this one.
Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v2 3/5] t3034: test option to disable renames

Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
 t/t3034-merge-recursive-rename-options.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
Show changes to t/t3034-merge-recursive-rename-options.sh +28 −0
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index 7ae7f83..a459236 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -68,6 +68,14 @@ check_find_renames_100 () {
 	rename_undetected d
 }
 
+check_no_renames () {
+	check_common &&
+	rename_undetected a &&
+	rename_undetected b &&
+	rename_undetected c &&
+	rename_undetected d
+}
+
 test_expect_success setup '
 	cat <<-\EOF >a-old &&
 	aa1
@@ -126,12 +134,30 @@ test_expect_success 'rename threshold is truncated' '
 	check_find_renames_100
 '
 
+test_expect_success 'disabled rename detection' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
 test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' '
 	git read-tree --reset -u HEAD &&
 	test_must_fail git merge-recursive --find-renames=25 --find-renames=75 HEAD^ -- HEAD master &&
 	check_find_renames_75
 '
 
+test_expect_success 'last wins in --no-renames --find-renames' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --no-renames --find-renames HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
+test_expect_success 'last wins in --find-renames --no-renames' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --find-renames --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
 test_expect_success 'assumption for further tests: trivial merge succeeds' '
 	git read-tree --reset -u HEAD &&
 	git merge-recursive HEAD -- HEAD HEAD &&
@@ -141,6 +167,8 @@ test_expect_success 'assumption for further tests: trivial merge succeeds' '
 	git merge-recursive --find-renames=75 HEAD -- HEAD HEAD &&
 	git diff --quiet --cached &&
 	git merge-recursive --find-renames=100% HEAD -- HEAD HEAD &&
+	git diff --quiet --cached &&
+	git merge-recursive --no-renames HEAD -- HEAD HEAD &&
 	git diff --quiet --cached
 '
 
-- 
2.7.1.492.gd821b20
Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v2 4/5] t3034: test deprecated interface

--find-renames= and --rename-threshold= should be aliases.
Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
Now includes tests with invalid arguments to --rename-threshold=
 t/t3034-merge-recursive-rename-options.sh | 42 +++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
Show changes to t/t3034-merge-recursive-rename-options.sh +42 −0
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index a459236..d4f9742 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -184,4 +184,46 @@ test_expect_success '--find-renames rejects non-numbers' '
 	git diff --quiet --cached
 '
 
+test_expect_success 'rename-threshold=<n> is a synonym for find-renames=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
+test_expect_success 'last wins in --no-renames --rename-threshold=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --no-renames --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
+test_expect_success 'last wins in --rename-threshold=<n> --no-renames' '
+	git read-tree --reset -u HEAD &&
+	git merge-recursive --rename-threshold=25 --no-renames HEAD^ -- HEAD master &&
+	check_no_renames
+'
+
+test_expect_success '--rename-threshold=<n> rejects negative argument' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=-25 HEAD -- HEAD HEAD &&
+	git diff --quiet --cached
+'
+
+test_expect_success '--rename-threshold=<n> rejects non-numbers' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=0xf HEAD -- HEAD HEAD &&
+	git diff --quiet --cached
+'
+
+test_expect_success 'last wins in --rename-threshold=<m> --find-renames=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --rename-threshold=25 --find-renames=75 HEAD^ -- HEAD master &&
+	check_find_renames_75
+'
+
+test_expect_success 'last wins in --find-renames=<m> --rename-threshold=<n>' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=75 --rename-threshold=25 HEAD^ -- HEAD master &&
+	check_find_renames_25
+'
+
 test_done
-- 
2.7.1.492.gd821b20
Felipe Gonçalves Assis· Feb 21, 2016, 22:59 UTC · re: Felipe Gonçalves Assis · lore

[PATCH v2 5/5] merge-recursive: find-renames resets threshold

Make the find-renames option follow the behaviour in git-diff, where it resets the threshold when none is given. So, for instance, "--find-renames=25 --find-renames" should result in the default threshold (50%) instead of 25%.

Add corresponding test.
Signed-off-by: Felipe Gonçalves Assis <felipegassis@gmail.com>
---
Now bundles the relevant test.
 merge-recursive.c                         | 4 +++-
 t/t3034-merge-recursive-rename-options.sh | 6 ++++++
 2 files changed, 9 insertions(+), 1 deletion(-)
Show changes to 2 files +9 −1

merge-recursive.c, t/t3034-merge-recursive-rename-options.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index 7bff5be..b880ae5 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2094,8 +2094,10 @@ int parse_merge_opt(struct merge_options *o, const char *s)
 		o->renormalize = 0;
 	else if (!strcmp(s, "no-renames"))
 		o->detect_rename = 0;
-	else if (!strcmp(s, "find-renames"))
+	else if (!strcmp(s, "find-renames")) {
 		o->detect_rename = 1;
+		o->rename_score = 0;
+	}
 	else if (skip_prefix(s, "find-renames=", &arg) ||
 		 skip_prefix(s, "rename-threshold=", &arg)) {
 		if ((o->rename_score = parse_rename_score(&arg)) == -1 || *arg != 0)
diff --git a/t/t3034-merge-recursive-rename-options.sh b/t/t3034-merge-recursive-rename-options.sh
index d4f9742..54d5f1b 100755
--- a/t/t3034-merge-recursive-rename-options.sh
+++ b/t/t3034-merge-recursive-rename-options.sh
@@ -146,6 +146,12 @@ test_expect_success 'last wins in --find-renames=<m> --find-renames=<n>' '
 	check_find_renames_75
 '
 
+test_expect_success '--find-renames resets threshold' '
+	git read-tree --reset -u HEAD &&
+	test_must_fail git merge-recursive --find-renames=25 --find-renames HEAD^ -- HEAD master &&
+	check_find_renames_50
+'
+
 test_expect_success 'last wins in --no-renames --find-renames' '
 	git read-tree --reset -u HEAD &&
 	test_must_fail git merge-recursive --no-renames --find-renames HEAD^ -- HEAD master &&
-- 
2.7.1.492.gd821b20
Junio C Hamano· Feb 22, 2016, 21:00 UTC · re: Felipe Gonçalves Assis · lore

Re: [PATCH v2 0/5] Tests and fixes for merge-recursive rename options

"Felipe Gonçalves Assis"  <felipeg.assis@gmail.com> writes:
Show 26 quoted lines
> This is a reorganisation of the previous series, bundling the test for the fix
> along with the commit itself, as suggested by Eric. It also includes many fixes
> and improvements pointed out by the same reviewer, whom I thank.
>
> The typo fix is the same as before.
>
> In "add rename threshold tests", I include tests involving --find-renames,
> except for one that depends on the fix.
>
> "test option to disable renames" adds tests involving --rename-threshold.
>
> "test deprecated interface" tests the aliasing --rename-threshold.
>
> In "find-renames resets threshold", the specific test for the feature was
> bundled along.
>
> To Junio: Please pay special attention to the test of threshold truncation.
> Given that it seems to be an undocumented feature, I am not sure whether it
> should be included or not.
>
> Felipe Gonçalves Assis (5):
>   merge-strategies.txt: fix typo
>   t3034: add rename threshold tests
>   t3034: test option to disable renames
>   t3034: test deprecated interface
>   merge-recursive: find-renames resets threshold

As I said, I am reluctant to take the 25%/50%/75% tests in their current form. Let me take the first one and a half of the last one (i.e. excluding the test) for now.

Thanks.
Felipe Gonçalves Assis· Feb 22, 2016, 22:16 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/5] Tests and fixes for merge-recursive rename options

On 22 February 2016 at 18:00, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> "Felipe Gonçalves Assis"  <felipeg.assis@gmail.com> writes:
>
>> This is a reorganisation of the previous series, bundling the test for the fix
>> along with the commit itself, as suggested by Eric. It also includes many fixes
>> and improvements pointed out by the same reviewer, whom I thank.
>>
>> The typo fix is the same as before.
>>
>> In "add rename threshold tests", I include tests involving --find-renames,
>> except for one that depends on the fix.
>>
>> "test option to disable renames" adds tests involving --rename-threshold.
>>
>> "test deprecated interface" tests the aliasing --rename-threshold.
>>
>> In "find-renames resets threshold", the specific test for the feature was
>> bundled along.
>>
>> To Junio: Please pay special attention to the test of threshold truncation.
>> Given that it seems to be an undocumented feature, I am not sure whether it
>> should be included or not.
>>
>> Felipe Gonçalves Assis (5):
>>   merge-strategies.txt: fix typo
>>   t3034: add rename threshold tests
>>   t3034: test option to disable renames
>>   t3034: test deprecated interface
>>   merge-recursive: find-renames resets threshold
>
> As I said, I am reluctant to take the 25%/50%/75% tests in their
> current form.  Let me take the first one and a half of the last one
> (i.e. excluding the test) for now.
>
> Thanks.

Ok, should I post a new version of the patch without the tests while I rework them or does that mean that you have already filtered them out locally?

Junio C Hamano· Feb 22, 2016, 22:29 UTC · re: Felipe Gonçalves Assis · lore

Re: [PATCH v2 0/5] Tests and fixes for merge-recursive rename options

Felipe Gonçalves Assis <felipeg.assis@gmail.com> writes:
Show 9 quoted lines
>> As I said, I am reluctant to take the 25%/50%/75% tests in their
>> current form.  Let me take the first one and a half of the last one
>> (i.e. excluding the test) for now.
>>
>> Thanks.
>
> Ok, should I post a new version of the patch without the tests while I
> rework them or does that mean that you have already filtered them out
> locally?
I already have and queued them tentatively as 
    c443d39 merge-recursive: find-renames resets threshold
    83837ec merge-strategies.txt: fix typo

but they haven't been merged to 'next', so it is up to you whether you rebuild the remainder on top of c443d39 or redo these 5 patches altogether (just tell me to drop these two if you go the latter route).

Thanks.
Felipe Gonçalves Assis· Feb 22, 2016, 22:37 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/5] Tests and fixes for merge-recursive rename options

On 22 February 2016 at 19:29, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Felipe Gonçalves Assis <felipeg.assis@gmail.com> writes:
>
>>> As I said, I am reluctant to take the 25%/50%/75% tests in their
>>> current form.  Let me take the first one and a half of the last one
>>> (i.e. excluding the test) for now.
>>>
>>> Thanks.
>>
>> Ok, should I post a new version of the patch without the tests while I
>> rework them or does that mean that you have already filtered them out
>> locally?
>
> I already have and queued them tentatively as
>
>     c443d39 merge-recursive: find-renames resets threshold
>     83837ec merge-strategies.txt: fix typo
>
> but they haven't been merged to 'next', so it is up to you whether
> you rebuild the remainder on top of c443d39 or redo these 5 patches
> altogether (just tell me to drop these two if you go the latter
> route).
>

My original patch started with exactly those commits and then added the tests, so it is fine for me.

The motivation for the reorganisation was bundling the relevant test with the fix, but then it might be better to publish the fix soon, right?

← back to recent threads