Re: [PATCH v3 1/7] builtin.h: take over documentation from api-builtin.txt

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

Martin Ågren <martin.agren@gmail.com> writes:

> diff --git a/builtin.h b/builtin.h
> index 498ac80d0..8d87d06da 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -6,6 +6,86 @@
>  #include "cache.h"
>  #include "commit.h"
>  
> +/*
> + * builtin API
> + * ===========
> + *
> + * Adding a new built-in
> + * ---------------------
> + *
> + * There are 4 things to do to add a built-in command implementation to
> + * Git:
> + *
> + * . Define the implementation of the built-in command `foo` with
> + *   signature:
> + *
> + *	int cmd_foo(int argc, const char **argv, const char *prefix);
> + *
> + * . Add the external declaration for the function to `builtin.h`.
> + *
> + * . Add the command to the `commands[]` table defined in `git.c`.
> + *   The entry should look like:
> + *
> + *	{ "foo", cmd_foo, <options> },
> + *
> + * where options is the bitwise-or of:
> + *
> + * `RUN_SETUP`:
> + *	If there is not a Git directory to work on, abort.  If there
> + *	is a work tree, chdir to the top of it if the command was
> + *	invoked in a subdirectory.  If there is no work tree, no
> + *	chdir() is done.
> + *
> + * `RUN_SETUP_GENTLY`:
> + *	If there is a Git directory, chdir as per RUN_SETUP, otherwise,
> + *	don't chdir anywhere.
> + *
> + * `USE_PAGER`:
> + *
> + *	If the standard output is connected to a tty, spawn a pager and
> + *	feed our output to it.
> + *
> + * `NEED_WORK_TREE`:
> + *
> + *	Make sure there is a work tree, i.e. the command cannot act
> + *	on bare repositories.
> + *	This only makes sense when `RUN_SETUP` is also set.
> + *
> + * `SUPPORT_SUPER_PREFIX`:
> + *
> + *	The built-in supports `--super-prefix`.
> + *
> + * . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.

Not a new problem but it will become much easier to follow if we
moved this item between the "implement cmd_foo()" and "declare
cmd_foo in builtin.h", like so:

 . Define the implementation of the built-in command `foo` with
   signature:

	int cmd_foo(int argc, const char **argv, const char *prefix);

   in a new file `builtin/foo.c`.

 . Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.

Alternatively, we may merge these two into one item (i.e. "in a new
file `builtin/foo.c` and add `builtin/foo.o` to ...").

But of course, this patch 1/7 should not do any of the above.  I am
suggesting a possible future clean-up for anybody on the list
listening from sidelines, and you do not have to be the person who
does it.

Thanks.