Re: [PATCH] Modify git-rev-list ... in merge order [ repost with bug fixes ]

From: Jon Seymour <jon.seymour@gmail.com>
Date: Jun 6, 2005, 01:09 UTC
Message-ID: <2cfc4032050605180958fcf395@mail.gmail.com>
In-Reply-To: <Pine.LNX.4.58.0506051741190.1876@ppc970.osdl.org>

On 6/6/05, Linus Torvalds <torvalds@osdl.org> wrote:

Show 18 quoted lines

> 
> 
> On Sun, 5 Jun 2005 jon@blackcubes.dyndns.org wrote:
> >
> > -static void show_commit(struct commit *commit)
> > +static int show_commit(struct commit *commit)
> 
> Ick. You've mixed "show_commit()" to be three totally independent things
>  - deciding whether to show at all
>  - showing the commit in traditional format
>  - showing the commit tree in the new "break" format
> 
> I really hate functions that do totally unrelated things, and I'm so much
> happier with the new show_commit() than the old "everything in one big
> function" thing, that I'm unhappy about mixing the thing up again.
> 
> Please leave show_commit() to just show the commit, and make the other
> decisions be independent of that.

My rationale was to re-use both the filtering logic currently in the show_commit_list while loop and the display logic, since I need both in order to maximise compatibility with the standard algorithm.

However, I understand your concerns.

My plan, therefore, is to split the filtering logic from show_commit_list's while loop into a separate function and create a third function which calls the filtering logic, then show_commit. I'll pass a pointer to the third function to sort_list_in_merge_order. I can leave the show_breaks functionality in show_commit (it is, after all, display functionality) or I can move it into the third function.

Is that ok by you?

jon.