git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [RFC PATCH v2 05/13] walken: configure rev_info and prepare for walk
Date: Thu, 27 Jun 2019 01:06:32 -0400	[thread overview]
Message-ID: <CAPig+cRduaOcVyrvhXc2moKSUsZa7-7HLh197xYT035HTQp61w@mail.gmail.com> (raw)
In-Reply-To: <20190626235032.177551-6-emilyshaffer@google.com>

On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
> `struct rev_info` is what's used by the struct itself.

What "struct itself"? Do you mean 'struct rev_info' is used by the
_walk_ itself? Or something?

> `repo_init_revisions()` initializes the struct; then we need to set it
> up for the walk we want to perform, which is done in
> `final_rev_info_setup()`.
> [...]
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> diff --git a/builtin/walken.c b/builtin/walken.c
> @@ -30,6 +31,40 @@ static void init_walken_defaults(void)
> +/*
> + * cmd_log calls a second set of init after the repo_init_revisions call. We'll
> + * mirror those settings in post_repo_init_init.
> + */

What is 'post_repo_init_init'?

I found the reference to cmd_log() confusing because I was looking for
it in this patch (as if it was being introduced here). Newcomers might
be even more confused. Perhaps if you state explicitly that you're
referring to existing code in an existing file, it might be clearer.
Maybe:

    builtin/log.c:cmd_log() calls a second ...

Overall, I find this entire function comment mystifying.

> +static void final_rev_info_setup(int argc, const char **argv, const char *prefix,
> +               struct rev_info *rev)
> +{
> +       /*
> +        * Optional:
> +        * setup_revision_opt is used to pass options to the setup_revisions()
> +        * call. It's got some special items for submodules and other types of
> +        * optimizations, but for now, we'll just point it to HEAD and call it
> +        * good. First we should make sure to reset it. This is useful for more
> +        * complicated stuff but a decent shortcut for the first pass is
> +        * add_head_to_pending().
> +        */

I had to pause over "call it good" for several seconds (since I
couldn't understand why someone would want to write "bad" code) until
I figured out you meant "do nothing else". It would be clearer simply
to drop that, ending the sentence at "HEAD":

    ..., but for now, we'll just point it at HEAD.

> +       /*
> +        * struct setup_revision_opt opt;
> +
> +        * memset(&opt, 0, sizeof(opt));
> +        * opt.def = "HEAD";
> +        * opt.revarg_opt = REVARG_COMMITTISH;
> +        * setup_revisions(argc, argv, rev, &opt);
> +        */
> +
> +       /* Let's force oneline format. */
> +       get_commit_format("oneline", rev);
> +       rev->verbose_header = 1;
> +
> +       /* add the HEAD to pending so we can start */
> +       add_head_to_pending(rev);
> +}

It would be easier for the reader to associate the
add_head_to_pending() invocation with the commented-out setting of
"HEAD" via 'setup_revision_opt' if the two bits abutted one another
without being separated by the "oneline" gunk.

  reply	other threads:[~2019-06-27  5:06 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-07  1:07 [PATCH] documentation: add tutorial for revision walking Emily Shaffer
2019-06-07  1:07 ` [RFC PATCH 00/13] example implementation of revwalk tutorial Emily Shaffer
2019-06-07  1:07   ` [RFC PATCH 01/13] walken: add infrastructure for revwalk demo Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 02/13] walken: add usage to enable -h Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 03/13] walken: add placeholder to initialize defaults Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 04/13] walken: add handler to git_config Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 05/13] walken: configure rev_info and prepare for walk Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 06/13] walken: perform our basic revision walk Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 07/13] walken: filter for authors from gmail address Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 08/13] walken: demonstrate various topographical sorts Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 09/13] walken: demonstrate reversing a revision walk list Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 10/13] walken: add unfiltered object walk from HEAD Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 11/13] walken: add filtered object walk Emily Shaffer
2019-06-07 19:15     ` Jeff Hostetler
2019-06-17 20:30       ` Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 12/13] walken: count omitted objects Emily Shaffer
2019-06-07  1:08   ` [RFC PATCH 13/13] walken: reverse the object walk order Emily Shaffer
2019-06-07  6:21 ` [PATCH] documentation: add tutorial for revision walking Eric Sunshine
2019-06-10 21:26   ` Junio C Hamano
2019-06-10 21:38     ` Eric Sunshine
2019-06-17 23:19   ` Emily Shaffer
2019-06-19  8:13     ` Eric Sunshine
2019-06-19 23:35       ` Emily Shaffer
2019-06-23 18:54         ` Eric Sunshine
2019-06-10 20:25 ` Junio C Hamano
2019-06-17 23:50   ` Emily Shaffer
2019-06-19 15:17     ` Junio C Hamano
2019-06-20 21:06       ` Emily Shaffer
2019-07-13  0:39         ` Josh Steadmon
2019-07-16  0:06           ` Emily Shaffer
2019-07-16 17:24             ` Junio C Hamano
2019-06-10 20:49 ` Junio C Hamano
2019-06-17 23:33   ` Emily Shaffer
2019-06-26 23:49 ` [PATCH v2] " Emily Shaffer
2019-06-26 23:50   ` [RFC PATCH v2 00/13] example implementation of revwalk tutorial Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 01/13] walken: add infrastructure for revwalk demo Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 02/13] walken: add usage to enable -h Emily Shaffer
2019-06-27  4:47       ` Eric Sunshine
2019-06-27 18:40         ` Emily Shaffer
2019-06-27  4:50       ` Eric Sunshine
2019-06-26 23:50     ` [RFC PATCH v2 03/13] walken: add placeholder to initialize defaults Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 04/13] walken: add handler to git_config Emily Shaffer
2019-06-27  4:54       ` Eric Sunshine
2019-06-27 18:47         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 05/13] walken: configure rev_info and prepare for walk Emily Shaffer
2019-06-27  5:06       ` Eric Sunshine [this message]
2019-06-27 18:56         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 06/13] walken: perform our basic revision walk Emily Shaffer
2019-06-27  5:16       ` Eric Sunshine
2019-06-27 20:54         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 07/13] walken: filter for authors from gmail address Emily Shaffer
2019-06-27  5:20       ` Eric Sunshine
2019-06-27 20:58         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 08/13] walken: demonstrate various topographical sorts Emily Shaffer
2019-06-27  5:22       ` Eric Sunshine
2019-06-27 22:12         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 09/13] walken: demonstrate reversing a revision walk list Emily Shaffer
2019-06-27  5:26       ` Eric Sunshine
2019-06-27 22:20         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 10/13] walken: add unfiltered object walk from HEAD Emily Shaffer
2019-06-27  5:37       ` Eric Sunshine
2019-06-27 22:31         ` Emily Shaffer
2019-06-28  0:48           ` Eric Sunshine
2019-07-01 19:19             ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 11/13] walken: add filtered object walk Emily Shaffer
2019-06-27  5:42       ` Eric Sunshine
2019-06-27 22:33         ` Emily Shaffer
2019-06-26 23:50     ` [RFC PATCH v2 12/13] walken: count omitted objects Emily Shaffer
2019-06-27  5:44       ` Eric Sunshine
2019-06-26 23:50     ` [RFC PATCH v2 13/13] walken: reverse the object walk order Emily Shaffer
2019-06-27 22:56     ` [RFC PATCH v2 00/13] example implementation of revwalk tutorial Emily Shaffer
2019-07-01 20:19   ` [PATCH v3] documentation: add tutorial for revision walking Emily Shaffer
2019-07-01 20:20     ` [RFC PATCH v3 00/13] example implementation of revwalk tutorial Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 01/13] walken: add infrastructure for revwalk demo Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 02/13] walken: add usage to enable -h Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 03/13] walken: add placeholder to initialize defaults Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 04/13] walken: add handler to git_config Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 05/13] walken: configure rev_info and prepare for walk Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 06/13] walken: perform our basic revision walk Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 07/13] walken: filter for authors from gmail address Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 08/13] walken: demonstrate various topographical sorts Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 09/13] walken: demonstrate reversing a revision walk list Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 10/13] walken: add unfiltered object walk from HEAD Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 11/13] walken: add filtered object walk Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 12/13] walken: count omitted objects Emily Shaffer
2019-07-01 20:20       ` [RFC PATCH v3 13/13] walken: reverse the object walk order Emily Shaffer
2019-07-25  9:25       ` [RFC PATCH v3 00/13] example implementation of revwalk tutorial Johannes Schindelin
2019-08-06 23:13         ` Emily Shaffer
2019-08-08 19:19           ` Johannes Schindelin
2019-07-24 23:11     ` [PATCH v3] documentation: add tutorial for revision walking Josh Steadmon
2019-07-24 23:32     ` Jonathan Tan
2019-08-06 23:10       ` Emily Shaffer
2019-08-06 23:19     ` [PATCH v4] " Emily Shaffer
2019-08-07 19:19       ` Junio C Hamano
2019-08-14 18:33         ` Emily Shaffer
2019-08-14 19:18           ` Junio C Hamano
2019-10-10 15:19       ` [PATCH v5] documentation: add tutorial for object walking Emily Shaffer
2019-10-11  5:50         ` Junio C Hamano
2019-10-11 23:26           ` Emily Shaffer
2019-10-11 17:50         ` SZEDER Gábor
2019-10-11 23:33           ` Emily Shaffer
2019-10-11 23:55         ` [PATCH v6] " Emily Shaffer

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAPig+cRduaOcVyrvhXc2moKSUsZa7-7HLh197xYT035HTQp61w@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).