public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Paul Tan <pyokagan@gmail•com>
Cc: git@vger•kernel.org, Stefan Beller <sbeller@google•com>,
	Johannes Schindelin <johannes.schindelin@gmx•de>
Subject: Re: [PATCH 0/2] teach git pull to handle --log=<n>
Date: Thu, 14 May 2015 10:11:37 -0700	[thread overview]
Message-ID: <xmqqd22384om.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1431508661-21729-1-git-send-email-pyokagan@gmail.com> (Paul Tan's message of "Wed, 13 May 2015 17:17:39 +0800")

Paul Tan <pyokagan@gmail•com> writes:

> Since efb779f (merge, pull: add '--(no-)log' command line option,
> 2008-04-06) git-pull supported the (--no-)log switch and would pass it
> to git-merge.
>
> 96e9420 (merge: Make '--log' an integer option for number of shortlog
> entries, 2010-09-08) implemented support for the --log=<n> switch, which
> would explicitly set the number of shortlog entries. However, git-pull
> does not recognize this option, and will instead pass it to git-fetch,
> leading to "unknown option" errors.
>
> This patch series implements a failing test that demonstrates the above,
> and teaches git-pull to handle the switch --log=<n>.

Looks good.

One advice; for a small patch like this one (and the "pull.ff vs
merge.ff" one, too), it is not necessary or even desirable to do a
two-step "first add a failure test and then another patch to fix and
flip the expectation" series.  Just do the fix and add a test to
expect success.

After all, the primary reason why we add test is *not* for you to
demonstrate that what you did works as expected.  It is to catch
other people breaking what you did in the future.

Thanks.

next prev parent reply	other threads:[~2015-05-14 17:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13  9:17 [PATCH 0/2] teach git pull to handle --log=<n> Paul Tan
2015-05-13  9:17 ` [PATCH 1/2] t5524: test --log=1 limits shortlog length Paul Tan
2015-05-13  9:17 ` [PATCH 2/2] pull: handle --log=<n> Paul Tan
2015-05-13  9:38   ` Matthieu Moy
2015-05-13 12:23     ` Johannes Schindelin
2015-05-13 20:03       ` Matthieu Moy
2015-05-13  9:40 ` [PATCH 0/2] teach git pull to " Matthieu Moy
2015-05-14 17:11 ` Junio C Hamano [this message]
2015-05-15 11:15   ` Paul Tan

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqd22384om.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=johannes.schindelin@gmx$(echo .)de \
    --cc=pyokagan@gmail$(echo .)com \
    --cc=sbeller@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox