Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
> +My First Revision Walk
> +======================
> +
> +== What's a Revision Walk?
> +
> +The revision walk is a key concept in Git - this is the process that underpins
> +operations like `git log`, `git blame`, and `git reflog`. Beginning at HEAD, the
> +list of objects is found by walking parent relationships between objects. The
> +revision walk can also be usedto determine whether or not a given object is
> +reachable from the current HEAD pointer.
s/usedto/used to/;
> +We'll put our fiddling into a new command. For fun, let's name it `git walken`.
> +Open up a new file `builtin/walken.c` and set up the command handler:
> +
> +----
> +/*
> + * "git walken"
> + *
> + * Part of the "My First Revision Walk" tutorial.
> + */
> +
> +#include <stdio.h>
Bad idea. In the generic part of the codebase, system headers are
supposed to be supplied by including git-compat-util.h (or cache.h
or builtin.h, that are common header files that begin by including
it and are allowed by CodingGuidelines to be used as such).
> +#include "builtin.h"
> +
> +int cmd_walken(int argc, const char **argv, const char *prefix)
> +{
> + printf(_("cmd_walken incoming...\n"));
> + return 0;
> +}
> +----
I wonder if it makes sense to use trace instead of printf, as our
reader has already seen the psuh example for doing the above.
> +Add usage text and `-h` handling, in order to pass the test suite:
It is not wrong per-se, and it indeed is a very good practice to
make sure that our subcommands consistently gives usage text and
short usage. Encouraging them early is a good idea.
But "in order to pass the test suite" invites "eh, the test suite
does not pass without usage and -h? why?".
Either drop the mention of "the test suite", or perhaps say
something like
Add usage text and `-h` handling, like all the subcommands
should consistently do (our test suite will notice and
complain if you fail to do so).
i.e. the real purpose is consistency and usability; test suite is
merely an enforcement mechanism.
> +----
> +{ "walken", cmd_walken, RUN_SETUP },
> +----
> +
> +Add it to the `Makefile` near the line for `builtin\worktree.o`:
Backslash intended?
