On 03/22/2015 11:40 PM, Eric Sunshine wrote: > On Sun, Mar 22, 2015 at 6:07 PM, Koosha Khajehmoogahi <koosha@xxxxxxxxx> wrote: >> On 03/22/2015 08:57 PM, Torsten Bögershausen wrote: >>> On 22.03.15 19:28, Koosha Khajehmoogahi wrote: >>>> Signed-off-by: Koosha Khajehmoogahi <koosha@xxxxxxxxx> >>>> --- >>>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh >>>> index 5f2b290..ab6f371 100755 >>>> --- a/t/t4202-log.sh >>>> +++ b/t/t4202-log.sh >>>> @@ -428,6 +428,147 @@ cat > expect <<\EOF >>>> * initial >>>> EOF >>>> >>>> +test_expect_success 'log with config log.merges=show' ' >>>> + git config log.merges show && >>>> + git log --pretty=tformat:%s >actual && >>>> + test_cmp both_commits_merges actual && >>>> + git config --unset log.merges >>> Do we need the unset here? >>> The log.merges is nicely set up before each test case, so can we drop the unset lines ? >>> (Or do I miss something ?) >> >> Good point; we can drop only those unset lines whose next test sets the log.merges. >> However, if the next test does not set it, we must unset it as it affects the >> default behavior of git-log. > > Such an approach would be too fragile. Tests may be re-ordered, added, > or removed. Better is to make each test as self-contained as possible. > See my review[1] of this patch for alternate suggestions. > > [1]: http://article.gmane.org/gmane.comp.version-control.git/266099 > That's why I wrote them this way actually. Also, thanks for your review. I will refactor my code to consider your suggestions. Thanks. -- Koosha -- 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