On Wed, Jun 26, 2019 at 7:51 PM Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxx>
> ---
> 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.
> + }
> +}> @@ -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).
