Matthieu, Ok, I'll fix that. I think I can also add tests, I can look at the tests for rev-list --count, with the understanding that I saw somebody else had made changes for the --use-bitmap-index option, and I am basing off of master for this, and thus don't feel comfortable with --use-bitmap-index at this time. If it's acceptable practice, I'll just squash everything I do on this feature and it's tests into one commit with a more detailed comment, and send the patch for that. I wasn't sure about how much history I should save, and how much I should split stuff up, so I appreciate your clarification. Thank you for your time, Lawrence Siebert On Fri, Jul 3, 2015 at 12:34 AM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > Lawrence Siebert <lawrencesiebert@xxxxxxxxx> writes: > >> added test comparing output between git log --count HEAD and >> git rev-list --count HEAD > > Unless there is a very long list of tests, I'd rather see this squashed > with PATCH 2/4. As a reviewer I prefer having code and tests in the same > place. > >> Signed-off-by: Lawrence Siebert <lawrencesiebert@xxxxxxxxx> >> --- >> t/t4202-log.sh | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/t/t4202-log.sh b/t/t4202-log.sh >> index 1b2e981..35f8d82 100755 >> --- a/t/t4202-log.sh >> +++ b/t/t4202-log.sh >> @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' ' >> test_must_fail git log --graph --no-walk >> ' >> >> +test_expect_success 'log --count' ' >> + git log --count HEAD > actual && >> + git rev-list --count HEAD > expect && > > The weird space is still there. > > Also, we write ">actual", not "> actual" in the Git coding style. > > That is actually a rather weak test. rev-list --count interacts with > --left-right, so I guess you want to test --count --left-right. > > Also, some revision-limiting options can reduce the count like > > git log --grep whatever > > and you should check that you actually count the right number here. > > (I don't know this part of the code enough, but I'm not sure you > actually deal with this properly) > > -- > Matthieu Moy > http://www-verimag.imag.fr/~moy/ -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.com -- 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