Re: [PATCH] docs: include "trace.h" in MyFirstObjectWalk.txt

[Junio C Hamano <gitster@pobox.com>]

Vinayak Dev <vinayakdev.sci@gmail.com> writes:

[jc: including Elijah, who owns a few topics of the recent past that
shuffled header files, to recipients].

> In Documentation/MyFirstObjectWalk.txt, the function
> trace_printf() is used to enable trace output.
> However, the file "trace.h" is not included, which
> produces an error when the code from the tutorial is
> compiled, with the compiler complaining that the 
> function is not defined before usage. Therefore, add
> an include for "trace.h" in the tutorial.
>
> Signed-off-by: Vinayak Dev <vinayakdev.sci@gmail.com>
> ---
>  Documentation/MyFirstObjectWalk.txt | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/MyFirstObjectWalk.txt b/Documentation/MyFirstObjectWalk.txt
> index eee513e86f..c3a23eb100 100644
> --- a/Documentation/MyFirstObjectWalk.txt
> +++ b/Documentation/MyFirstObjectWalk.txt
> @@ -41,6 +41,7 @@ Open up a new file `builtin/walken.c` and set up the command handler:
>   */
>  
>  #include "builtin.h"
> +#include "trace.h"
>  
>  int cmd_walken(int argc, const char **argv, const char *prefix)
>  {

OK.

> @@ -49,12 +50,13 @@ int cmd_walken(int argc, const char **argv, const char *prefix)
>  }
>  ----
>  
> -NOTE: `trace_printf()` differs from `printf()` in that it can be turned on or
> -off at runtime. For the purposes of this tutorial, we will write `walken` as
> -though it is intended for use as a "plumbing" command: that is, a command which
> -is used primarily in scripts, rather than interactively by humans (a "porcelain"
> -command). So we will send our debug output to `trace_printf()` instead. When
> -running, enable trace output by setting the environment variable `GIT_TRACE`.
> +NOTE: `trace_printf()`, defined in `trace.h`, differs from `printf()` in
> +that it can be turned on or off at runtime. For the purposes of this
> +tutorial, we will write `walken` as though it is intended for use as
> +a "plumbing" command: that is, a command which is used primarily in
> +scripts, rather than interactively by humans (a "porcelain" command).
> +So we will send our debug output to `trace_printf()` instead.
> +When running, enable trace output by setting the environment variable `GIT_TRACE`.

All of the above may be good currently, but as soon as somebody does
another round of header shuffling, we risk a very similar breakage.
It is a good time to think about ways to avoid that, while the pain
is fresh in our mind.

One "cop out" thing we can do to limit the damage may be to loosen
the text in the "NOTE:", so that it does *NOT* mention exact header
files the API functions appear and let the readers learn from the
source themselves, with "git grep" helping their way.  Or tone down
"defined in X" somehow to hint that these details may change.

More effective things that would involve higher implementation cost
(but will reduce maintenance cost) would be to actually make sure
that those who update the API will notice that they are breaking
these samples when they develop their series.  

In https://lore.kernel.org/git/xmqq1qhu9ifp.fsf@gitster.g/, I've
floated some strawman ideas but people may be able to invent better
ways.