[RFC,v2,09/13] walken: demonstrate reversing a revision walk list
diff mbox series

Message ID 20190626235032.177551-10-emilyshaffer@google.com
State New
Headers show
Series
  • example implementation of revwalk tutorial
Related show

Commit Message

Emily Shaffer June 26, 2019, 11:50 p.m. UTC
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.)

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 builtin/walken.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Eric Sunshine June 27, 2019, 5:26 a.m. UTC | #1
On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> 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.)

> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> 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;
Emily Shaffer June 27, 2019, 10:20 p.m. UTC | #2
On Thu, Jun 27, 2019 at 01:26:19AM -0400, Eric Sunshine wrote:
> On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> 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 <emilyshaffer@google.com>
> > ---
> > 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;

Patch
diff mbox series

diff --git a/builtin/walken.c b/builtin/walken.c
index 6cc451324a..958923c172 100644
--- 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;
 	
 	/* Let's play with the sort order. */
 	rev->topo_order = 1;