Re: [RFC PATCH v2 10/13] walken: add unfiltered object walk from HEAD
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jun 27, 2019, 05:37 UTC
- Message-ID
- <CAPig+cR7rHokaTtZcJJg8trJ14xO3hdeWHqLnB4aOnEUak051w@mail.gmail.com>
- In-Reply-To
- <20190626235032.177551-11-emilyshaffer@google.com>
On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@google.com> wrote:
Show 27 quoted lines
> Provide a demonstration of a revision walk which traverses all types of
> object, not just commits. This type of revision walk is used for
> operations such as creating packfiles and performing fetches or clones,
> so it's useful to teach new developers how it works. For starters, only
> demonstrate the unfiltered version, as this will make the tutorial
> easier to follow.
> [...]
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
> diff --git a/builtin/walken.c b/builtin/walken.c
> @@ -103,6 +109,65 @@ static int git_walken_config(const char *var, const char *value, void *cb)
> +static void walken_show_commit(struct commit *cmt, void *buf)
> +{
> + commit_count++;
> +}
> +
> +static void walken_show_object(struct object *obj, const char *str, void *buf)
> +{
> + switch (obj->type) {
> + [...]
> + case OBJ_TAG:
> + tag_count++;
> + break;
> + case OBJ_COMMIT:
> + die(_("unexpectedly encountered a commit in "
> + "walken_show_object\n"));
> + commit_count++;The "commit_count++" line is not only dead code, but it also confuses the reader (or makes the reader think that the author of this code doesn't understand C programming). You should drop this line.
> + break;
> + default:
> + die(_("unexpected object type %s\n"), type_name(obj->type));
> + break;Likewise, this "break" (and the one in the OBJ_COMMIT case) are dead code which should be dropped to avoid confusing the reader.
Don't localize the die() message via _() here or in the preceding OBJ_COMMIT case.
The two die() messages are unnecessarily dissimilar. Try to unify them so that they read in the same way.
Show 13 quoted lines
> + } > +}> @@ -113,15 +178,15 @@ static void walken_commit_walk(struct rev_info *rev) > /* > - * prepare_revision_walk() gets the final steps ready for a revision > + * prepare_revision_walk() gets the final steps ready for a revision > * walk. We check the return value for errors. > - */ > + */ > /* > - * Now we can start the real commit walk. get_revision grabs the next > + * Now we can start the real commit walk. get_revision grabs the next > * revision based on the contents of rev. > */
I think these are just correcting whitespace/indentation errors I pointed out in an earlier patch (so they are unnecessary noise in this patch).