Re: [PATCH/RFC v3 0/6] N-th last checked out branch

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 17, 2009, 05:52 UTC
Message-ID: <alpine.DEB.1.00.0901170646560.3586@pacific.mpi-cbg.de>
In-Reply-To: <1232163011-20088-1-git-send-email-trast@student.ethz.ch>

Hi,

On Sat, 17 Jan 2009, Thomas Rast wrote:

Show 12 quoted lines

>  Documentation/git-checkout.txt  |    4 +
>  Documentation/git-rev-parse.txt |    3 +
>  builtin-checkout.c              |   15 ++-
>  cache.h                         |    1 +
>  refs.c                          |  285 +++++++++++++++++++++++----------------
>  refs.h                          |    1 +
>  sha1_name.c                     |   79 +++++++++++-
>  t/t1505-rev-parse-last.sh       |   71 ++++++++++
>  t/t2012-checkout-last.sh        |   50 +++++++
>  9 files changed, 387 insertions(+), 122 deletions(-)
>  create mode 100755 t/t1505-rev-parse-last.sh
>  create mode 100755 t/t2012-checkout-last.sh

Let's quickly compare that to what Junio sent:

 builtin-checkout.c |   10 +++++-
 cache.h            |    1 +
 sha1_name.c        |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

Granted, the documentation and tests are nice, but why that big change in refs.c? After all, the more you change in one go, the more you can break.

Let's take one step back. The two things that Junio's patch left to be desired (in addition to documentation and tests) are

- reflogs are traversed twice.  While this is not a showstopper, it is 
  easily improved by building a string_list and then picking the one entry 
  we're interested in, and

- the '-' handling you seem to want.

I really have to ask: why did you not work on top of Junio's patch, just adding docs, tests, and checkout -? And then -- maybe -- the string_list...

Although I have to admit that I am not _that_ convinced the string_list is worth it: reflogs are not evaluated all the time, so it is definitely not performance critical.

Ciao, Dscho