Subject: Re: [RFC PATCH v2 09/13] walken: demonstrate reversing a revision walk list

From: Emily Shaffer <[email-masked]>
Date: Thu, 27 Jun 2019 15:20:32 -0700

On Thu, Jun 27, 2019 at 01:26:19AM -0400, Eric Sunshine wrote:
> On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <[email-masked]> wrote:
> > The final installment in the tutorial about sorting revision walk
> > outputs. This commit reverses the commit list, so that we see newer
> > commits last (handy since we aren't using a pager).
> >
> > It's important to note that rev->reverse needs to be set after
> > add_head_to_pending() or before setup_revisions(). (This is mentioned in
> > the accompanying tutorial.)
> 
> This leaves the reader wondering "why that requirement?". Is it
> because those functions may change the value or otherwise depend upon
> the value?
> 
> Also, something this important probably deserves an in-code comment
> (and need not be mentioned in the commit message if the in-code
> comment explains it well.)

This I will remove. I removed from the tutorial as it turned out I was
incorrect. Thanks.

> 
> > Signed-off-by: Emily Shaffer <[email-masked]>
> > ---
> > diff --git a/builtin/walken.c b/builtin/walken.c
> > @@ -69,6 +69,9 @@ static void final_rev_info_setup(int argc, const char **argv, const char *prefix
> >         /* add the HEAD to pending so we can start */
> >         add_head_to_pending(rev);
> > +
> > +       /* Reverse the order */
> > +       rev->reverse = 1;