Re: [PATCH v7 1/5] command-list.txt: prepare with [commands] header
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:47
On Fri, May 15, 2015 at 2:35 PM, Sébastien Guimmara [off-list ref] 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.
quoted hunk ↗ jump to hunk
Helped-by: Eric Sunshine [off-list ref] Signed-off-by: Sébastien Guimmara <redacted> ---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.
quoted hunk ↗ jump to hunk
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 coverdiff --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
quoted hunk ↗ jump to hunk
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' \
...
quoted hunk ↗ jump to hunk
-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