On Fri, May 15, 2015 at 2:35 PM, Sébastien Guimmara
<sebastien.guimmara@xxxxxxxxx> wrote:
> command-list.txt: prepare with [commands] header
>
> Add a [commands] header before the actual command list, then make the
> following files ignore this header in their parsing:
>
> * cmd-list.perl
> * Makefile (check-docks target)
When these patches are ultimately applied, they won't have the benefit
of the cover letter to explain the purpose of this change and,
unfortunately, this commit message is lacking in that respect. The
commit message should explain the reason you're making this change
("prepare machinery to deal with upcoming new common group
specification" or such). With a proper explanation, it's not necessary
to state (as you do above) precisely which bits of machinery you are
changing. The patch itself already does that well enough.
More below.
> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@xxxxxxxxx>
> ---
> diff --git a/Documentation/howto/new-command.txt b/Documentation/howto/new-command.txt
> index d7de5a3..6d772bd 100644
> --- a/Documentation/howto/new-command.txt
> +++ b/Documentation/howto/new-command.txt
> @@ -95,7 +95,9 @@ your language, document it in the INSTALL file.
> that categorizes commands by type, so they can be listed in appropriate
> subsections in the documentation's summary command list. Add an entry
> for yours. To understand the categories, look at git-commands.txt
> -in the main directory.
> +in the main directory. If the new command is part of the typical Git
> +workflow and you believe it common enough to be mentioned in 'git help',
> +map this command to a common group in the column [common].
I think you meant to fold this change into patch 2/5 where the
[common] block is actually added. It doesn't make much sense to talk
about it here before exists.
> 7. Give the maintainer one paragraph to include in the RelNotes file
> to describe the new feature; a good place to do so is in the cover
> diff --git a/Makefile b/Makefile
> index 25a453b..0cb2045 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2454,7 +2454,7 @@ check-docs::
> esac ; \
> test -f "Documentation/$$v.txt" || \
> echo "no doc: $$v"; \
> - sed -e '/^#/d' command-list.txt | \
> + sed -e '1,/^\[commands\]/d' -e '/^#/d' <command-list.txt | \
This is partly my fault since my original suggestion for how to skip
[common] and [commands] sections[1] was typed on-the-spot when
composing the mail message, without the context of looking at this
Makefile, but changing this invocation to use redirection
(<command-list.txt) rather than a simple argument (command-list.txt)
seems unwarranted. Worse, it may confuse readers into thinking that
there is some important but obscure reason for the change which they
cannot fathom. Also, the invocation in the hunk just below this one
does not bother with redirection, so use of redirection here seems
doubly mysterious.
[1]: http://article.gmane.org/gmane.comp.version-control.git/268598
> grep -q "^$$v[ ]" || \
> case "$$v" in \
> git) ;; \
> @@ -2463,6 +2463,7 @@ check-docs::
> done; \
> ( \
> sed -e '/^#/d' \
> + -e '1,/^\[commands\]/d' \
This won't work as expected. If you invoke 'check-docs', you'll see
this (undesired) output:
% make check-docs
removed but listed: [commands]
removed but listed: [common]
removed but listed: history
removed but listed: info
removed but listed: init
removed but listed: remote
removed but listed: worktree
%
which is the two [headers] you added plus all the new common group
tags. To fix, re-order the sed expressions so that 1,/^\[commands\]/d
comes first:
sed -e '1,/^\[commands\]/d' \
-e '/^#/d' \
...
> -e 's/[ ].*//' \
> -e 's/^/listed /' command-list.txt; \
> $(MAKE) -C Documentation print-man1 | \
> diff --git a/command-list.txt b/command-list.txt
> index 54d8d21..caed872 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -1,5 +1,6 @@
> # List of known git commands.
> # command name category [deprecated] [common]
> +[commands]
> git-add mainporcelain common
> git-am mainporcelain
> git-annotate ancillaryinterrogators
> --
> 2.4.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html