[PATCH v3 0/5] doc: asciidoc cleanups

[PATCH v3 0/5] doc: asciidoc cleanups

[Felipe Contreras]

This patch series is an attempt to cleanup the Makefile of the documentation.

Since v2 only certain $(RM) commands are moved from one patch to another.

Felipe Contreras (5):
  doc: refactor common asciidoc dependencies
  doc: improve asciidoc dependencies
  doc: remove unnecessary rm instances
  doc: simplify Makefile using .DELETE_ON_ERROR
  doc: avoid using rm directly

 Documentation/Makefile | 77 ++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 48 deletions(-)

Range-diff against v2:
1:  55b188c8ad = 1:  cbcaefed3e doc: refactor common asciidoc dependencies
2:  e69d0a5b89 = 2:  1c8b0b9123 doc: improve asciidoc dependencies
3:  4f18675ce9 < -:  ---------- doc: remove unnecessary rm instances
-:  ---------- > 3:  3da461a55d doc: remove unnecessary rm instances
4:  935675e070 ! 4:  a8c4c4ae26 doc: simplify Makefile using .DELETE_ON_ERROR
    @@ Documentation/Makefile: docdep_prereqs = \
      	cmd-list.made $(cmds_txt)
      
      doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
    --	$(QUIET_GEN)$(RM) $@+ $@ && \
    --	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
    +-	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
     -	mv $@+ $@
     +	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)
      
    @@ Documentation/Makefile: clean:
      	$(RM) GIT-ASCIIDOCFLAGS
      
      $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
    --	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
    --	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
    +-	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@+ $< && \
     -	mv $@+ $@
     +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@ $<
      
      $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
    --	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
    --	$(TXT_TO_HTML) -o $@+ $< && \
    +-	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@+ $< && \
     -	mv $@+ $@
     +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@ $<
      
    @@ Documentation/Makefile: manpage-base-url.xsl: manpage-base-url.xsl.in
      	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
      
      %.xml : %.txt $(ASCIIDOC_DEPS)
    --	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
    --	$(TXT_TO_XML) -d manpage -o $@+ $< && \
    +-	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@+ $< && \
     -	mv $@+ $@
     +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
      
      user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
    --	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
    --	$(TXT_TO_XML) -d book -o $@+ $< && \
    +-	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@+ $< && \
     -	mv $@+ $@
     +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@ $<
      
    @@ Documentation/Makefile: XSLTOPTS += --stringparam html.stylesheet docbook-xsl.cs
      XSLTOPTS += --param generate.consistent.ids 1
      
      user-manual.html: user-manual.xml $(XSLT)
    --	$(QUIET_XSLTPROC)$(RM) $@+ $@ && \
    --	xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
    +-	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
     -	mv $@+ $@
     +	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@ $(XSLT) $<
      
    @@ Documentation/Makefile: XSLTOPTS += --stringparam html.stylesheet docbook-xsl.cs
      	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi
      
      user-manual.texi: user-manual.xml
    --	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
    --	$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
    +-	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
     -	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
     -	rm $@++ && \
     -	mv $@+ $@
    @@ Documentation/Makefile: XSLTOPTS += --stringparam html.stylesheet docbook-xsl.cs
     +	rm $@+
      
      user-manual.pdf: user-manual.xml
    --	$(QUIET_DBLATEX)$(RM) $@+ $@ && \
    --	$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
    +-	$(QUIET_DBLATEX)$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
     -	mv $@+ $@
     +	$(QUIET_DBLATEX)$(DBLATEX) -o $@ $(DBLATEX_COMMON) $<
      
      gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
    --	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
    -+	$(QUIET_DB2TEXI) \
    + 	$(QUIET_DB2TEXI) \
      	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
      		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
     -		rm $(xml)+ &&) true) > $@++ && \
    @@ Documentation/Makefile: XSLTOPTS += --stringparam html.stylesheet docbook-xsl.cs
      	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
      
      $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
    --	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
    --	$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
    +-	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
     -	mv $@+ $@
     +	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@
      
      howto-index.txt: howto-index.sh $(HOWTO_TXT)
    --	$(QUIET_GEN)$(RM) $@+ $@ && \
    --	'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
    +-	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
     -	mv $@+ $@
     +	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@
      
      $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
      	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt
    -@@ Documentation/Makefile: WEBDOC_DEST = /pub/software/scm/git/docs
    - 
    - howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
    +@@ Documentation/Makefile: howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
      $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS
    --	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
    -+	$(QUIET_ASCIIDOC) \
    + 	$(QUIET_ASCIIDOC) \
      	sed -e '1,/^$$/d' $< | \
     -	$(TXT_TO_HTML) - >$@+ && \
     -	mv $@+ $@
5:  b621f3b8e9 = 5:  2c330987b5 doc: avoid using rm directly
-- 
2.32.0.rc0

[PATCH v3 1/5] doc: refactor common asciidoc dependencies

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2aae4c9cbb..46d9b98dac 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -139,6 +139,7 @@ ASCIIDOC_CONF = -f asciidoc.conf
 ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
 		-amanversion=$(GIT_VERSION) \
 		-amanmanual='Git Manual' -amansource='Git'
+ASCIIDOC_DEPS = asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
 TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
 MANPAGE_XSL = manpage-normal.xsl
@@ -354,12 +355,12 @@ clean:
 	$(RM) manpage-base-url.xsl
 	$(RM) GIT-ASCIIDOCFLAGS
 
-$(MAN_HTML): %.html : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+$(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
-$(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+$(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_HTML) -o $@+ $< && \
 	mv $@+ $@
@@ -371,7 +372,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
-%.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+%.xml : %.txt $(ASCIIDOC_DEPS)
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
 	$(TXT_TO_XML) -d manpage -o $@+ $< && \
 	mv $@+ $@
-- 
2.32.0.rc0

[PATCH v3 2/5] doc: improve asciidoc dependencies

[Felipe Contreras]

asciidoc needs asciidoc.conf, asciidoctor asciidoctor-extensions.rb.

Neither needs the other.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 46d9b98dac..0f59cc0853 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -139,7 +139,7 @@ ASCIIDOC_CONF = -f asciidoc.conf
 ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
 		-amanversion=$(GIT_VERSION) \
 		-amanmanual='Git Manual' -amansource='Git'
-ASCIIDOC_DEPS = asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+ASCIIDOC_DEPS = asciidoc.conf GIT-ASCIIDOCFLAGS
 TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
 TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
 MANPAGE_XSL = manpage-normal.xsl
@@ -194,6 +194,7 @@ ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
 ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+ASCIIDOC_DEPS = asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 DBLATEX_COMMON =
 XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
-- 
2.32.0.rc0

[PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Commits 50cff52f1a (When generating manpages, delete outdated targets
first., 2007-08-02) and f9286765b2 (Documentation/Makefile: remove
cmd-list.made before redirecting to it., 2007-08-06) created these rm
instances for a very rare corner-case: building as root by mistake.

It's odd to have workarounds here, but nowhere else in the Makefile--
which already fails in this stuation, starting from
Documentation/technical/.

We gain nothing but complexity, so let's remove them.

Comments-by: Jeff King <peff@peff.net>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 42 +++++++++++++++---------------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f59cc0853..73b88c3aad 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -296,8 +296,7 @@ docdep_prereqs = \
 	cmd-list.made $(cmds_txt)
 
 doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
-	$(QUIET_GEN)$(RM) $@+ $@ && \
-	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
+	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
 	mv $@+ $@
 
 ifneq ($(MAKECMDGOALS),clean)
@@ -318,8 +317,7 @@ cmds_txt = cmds-ancillaryinterrogators.txt \
 $(cmds_txt): cmd-list.made
 
 cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT)
-	$(QUIET_GEN)$(RM) $@ && \
-	$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
+	$(QUIET_GEN)$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
 	date >$@
 
 mergetools_txt = mergetools-diff.txt mergetools-merge.txt
@@ -327,7 +325,7 @@ mergetools_txt = mergetools-diff.txt mergetools-merge.txt
 $(mergetools_txt): mergetools-list.made
 
 mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
-	$(QUIET_GEN)$(RM) $@ && \
+	$(QUIET_GEN) \
 	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
 		. ../git-mergetool--lib.sh && \
 		show_tool_names can_diff "* " || :' >mergetools-diff.txt && \
@@ -357,30 +355,25 @@ clean:
 	$(RM) GIT-ASCIIDOCFLAGS
 
 $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
 $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_HTML) -o $@+ $< && \
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@+ $< && \
 	mv $@+ $@
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
-	$(QUIET_XMLTO)$(RM) $@ && \
-	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
 %.xml : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_XML) -d manpage -o $@+ $< && \
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@+ $< && \
 	mv $@+ $@
 
 user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-	$(TXT_TO_XML) -d book -o $@+ $< && \
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@+ $< && \
 	mv $@+ $@
 
 technical/api-index.txt: technical/api-index-skel.txt \
@@ -402,27 +395,24 @@ XSLTOPTS += --stringparam html.stylesheet docbook-xsl.css
 XSLTOPTS += --param generate.consistent.ids 1
 
 user-manual.html: user-manual.xml $(XSLT)
-	$(QUIET_XSLTPROC)$(RM) $@+ $@ && \
-	xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
+	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
 	mv $@+ $@
 
 git.info: user-manual.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi
 
 user-manual.texi: user-manual.xml
-	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
-	$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
 	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
 	rm $@++ && \
 	mv $@+ $@
 
 user-manual.pdf: user-manual.xml
-	$(QUIET_DBLATEX)$(RM) $@+ $@ && \
-	$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
+	$(QUIET_DBLATEX)$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
 	mv $@+ $@
 
 gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
-	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
+	$(QUIET_DB2TEXI) \
 	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
 		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
 		rm $(xml)+ &&) true) > $@++ && \
@@ -434,13 +424,11 @@ gitman.info: gitman.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
 
 $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
-	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
-	$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
 	mv $@+ $@
 
 howto-index.txt: howto-index.sh $(HOWTO_TXT)
-	$(QUIET_GEN)$(RM) $@+ $@ && \
-	'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
+	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
 	mv $@+ $@
 
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
@@ -450,7 +438,7 @@ WEBDOC_DEST = /pub/software/scm/git/docs
 
 howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
 $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS
-	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
+	$(QUIET_ASCIIDOC) \
 	sed -e '1,/^$$/d' $< | \
 	$(TXT_TO_HTML) - >$@+ && \
 	mv $@+ $@
-- 
2.32.0.rc0

[PATCH v3 4/5] doc: simplify Makefile using .DELETE_ON_ERROR

[Felipe Contreras]

Currently GNU make already removes files when catching an interruption
signal, however, in order to deal with other kinds of errors a
workaround is in place to store target output to a temporary file, and
only move it to its right place on success.

By enabling the built-in .DELETE_ON_ERROR we let make do this task, so
we don't have to.

This way the rules can be simplified a lot.

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 47 +++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 73b88c3aad..eaff97dcb8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -296,8 +296,7 @@ docdep_prereqs = \
 	cmd-list.made $(cmds_txt)
 
 doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
-	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
-	mv $@+ $@
+	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)
 
 ifneq ($(MAKECMDGOALS),clean)
 -include doc.dep
@@ -355,12 +354,10 @@ clean:
 	$(RM) GIT-ASCIIDOCFLAGS
 
 $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@ $<
 
 $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@ $<
 
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
@@ -369,12 +366,10 @@ manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
 
 %.xml : %.txt $(ASCIIDOC_DEPS)
-	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@ $<
 
 user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
-	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@+ $< && \
-	mv $@+ $@
+	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@ $<
 
 technical/api-index.txt: technical/api-index-skel.txt \
 	technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS))
@@ -395,41 +390,35 @@ XSLTOPTS += --stringparam html.stylesheet docbook-xsl.css
 XSLTOPTS += --param generate.consistent.ids 1
 
 user-manual.html: user-manual.xml $(XSLT)
-	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
-	mv $@+ $@
+	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@ $(XSLT) $<
 
 git.info: user-manual.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi
 
 user-manual.texi: user-manual.xml
-	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
-	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
-	rm $@++ && \
-	mv $@+ $@
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@+ && \
+	$(PERL_PATH) fix-texi.perl <$@+ >$@ && \
+	rm $@+
 
 user-manual.pdf: user-manual.xml
-	$(QUIET_DBLATEX)$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
-	mv $@+ $@
+	$(QUIET_DBLATEX)$(DBLATEX) -o $@ $(DBLATEX_COMMON) $<
 
 gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
 	$(QUIET_DB2TEXI) \
 	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
 		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
-		rm $(xml)+ &&) true) > $@++ && \
-	$(PERL_PATH) cat-texi.perl $@ <$@++ >$@+ && \
-	rm $@++ && \
-	mv $@+ $@
+		rm $(xml)+ &&) true) > $@+ && \
+	$(PERL_PATH) cat-texi.perl $@ <$@+ >$@ && \
+	rm $@+
 
 gitman.info: gitman.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
 
 $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
-	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
-	mv $@+ $@
+	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@
 
 howto-index.txt: howto-index.sh $(HOWTO_TXT)
-	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
-	mv $@+ $@
+	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@
 
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) $*.txt
@@ -440,8 +429,7 @@ howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
 $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC) \
 	sed -e '1,/^$$/d' $< | \
-	$(TXT_TO_HTML) - >$@+ && \
-	mv $@+ $@
+	$(TXT_TO_HTML) - >$@
 
 install-webdoc : html
 	'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(WEBDOC_DEST)
@@ -482,4 +470,7 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
+# Delete the target file on error
+.DELETE_ON_ERROR:
+
 .PHONY: FORCE
-- 
2.32.0.rc0

[PATCH v3 5/5] doc: avoid using rm directly

[Felipe Contreras]

That's what we have $(RM) for.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index eaff97dcb8..f5605b7767 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -398,7 +398,7 @@ git.info: user-manual.texi
 user-manual.texi: user-manual.xml
 	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@+ && \
 	$(PERL_PATH) fix-texi.perl <$@+ >$@ && \
-	rm $@+
+	$(RM) $@+
 
 user-manual.pdf: user-manual.xml
 	$(QUIET_DBLATEX)$(DBLATEX) -o $@ $(DBLATEX_COMMON) $<
@@ -407,9 +407,9 @@ gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
 	$(QUIET_DB2TEXI) \
 	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
 		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
-		rm $(xml)+ &&) true) > $@+ && \
+		$(RM) $(xml)+ &&) true) > $@+ && \
 	$(PERL_PATH) cat-texi.perl $@ <$@+ >$@ && \
-	rm $@+
+	$(RM) $@+
 
 gitman.info: gitman.texi
 	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
-- 
2.32.0.rc0

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Commits 50cff52f1a (When generating manpages, delete outdated targets
> first., 2007-08-02) and f9286765b2 (Documentation/Makefile: remove
> cmd-list.made before redirecting to it., 2007-08-06) created these rm
> instances for a very rare corner-case: building as root by mistake.
>
> It's odd to have workarounds here, but nowhere else in the Makefile--
> which already fails in this stuation, starting from
> Documentation/technical/.
>
> We gain nothing but complexity, so let's remove them.
>
> Comments-by: Jeff King <peff@peff.net>

This is curious.  I understand what other kinds of trailers like
Helped-by and Suggested-by people sometimes use mean, but this one
does not tell anything other than that this person had some comments
on an earlier rounds, does not tell us what kind of comments they
were and if something was done to address those comments or the
comments were totally ignored or what.

The patch text itself, and the justification given in the second
paragraph, both make perfect sense to me.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/Makefile | 42 +++++++++++++++---------------------------
>  1 file changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 0f59cc0853..73b88c3aad 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -296,8 +296,7 @@ docdep_prereqs = \
>  	cmd-list.made $(cmds_txt)
>  
>  doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
> -	$(QUIET_GEN)$(RM) $@+ $@ && \
> -	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
> +	$(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
>  	mv $@+ $@
>  
>  ifneq ($(MAKECMDGOALS),clean)
> @@ -318,8 +317,7 @@ cmds_txt = cmds-ancillaryinterrogators.txt \
>  $(cmds_txt): cmd-list.made
>  
>  cmd-list.made: cmd-list.perl ../command-list.txt $(MAN1_TXT)
> -	$(QUIET_GEN)$(RM) $@ && \
> -	$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
> +	$(QUIET_GEN)$(PERL_PATH) ./cmd-list.perl ../command-list.txt $(cmds_txt) $(QUIET_STDERR) && \
>  	date >$@
>  
>  mergetools_txt = mergetools-diff.txt mergetools-merge.txt
> @@ -327,7 +325,7 @@ mergetools_txt = mergetools-diff.txt mergetools-merge.txt
>  $(mergetools_txt): mergetools-list.made
>  
>  mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
> -	$(QUIET_GEN)$(RM) $@ && \
> +	$(QUIET_GEN) \
>  	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
>  		. ../git-mergetool--lib.sh && \
>  		show_tool_names can_diff "* " || :' >mergetools-diff.txt && \
> @@ -357,30 +355,25 @@ clean:
>  	$(RM) GIT-ASCIIDOCFLAGS
>  
>  $(MAN_HTML): %.html : %.txt $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_HTML) -d manpage -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -d manpage -o $@+ $< && \
>  	mv $@+ $@
>  
>  $(OBSOLETE_HTML): %.html : %.txto $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_HTML) -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_HTML) -o $@+ $< && \
>  	mv $@+ $@
>  
>  manpage-base-url.xsl: manpage-base-url.xsl.in
>  	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
>  
>  %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
> -	$(QUIET_XMLTO)$(RM) $@ && \
> -	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
> +	$(QUIET_XMLTO)$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
>  
>  %.xml : %.txt $(ASCIIDOC_DEPS)
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_XML) -d manpage -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d manpage -o $@+ $< && \
>  	mv $@+ $@
>  
>  user-manual.xml: user-manual.txt user-manual.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> -	$(TXT_TO_XML) -d book -o $@+ $< && \
> +	$(QUIET_ASCIIDOC)$(TXT_TO_XML) -d book -o $@+ $< && \
>  	mv $@+ $@
>  
>  technical/api-index.txt: technical/api-index-skel.txt \
> @@ -402,27 +395,24 @@ XSLTOPTS += --stringparam html.stylesheet docbook-xsl.css
>  XSLTOPTS += --param generate.consistent.ids 1
>  
>  user-manual.html: user-manual.xml $(XSLT)
> -	$(QUIET_XSLTPROC)$(RM) $@+ $@ && \
> -	xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
> +	$(QUIET_XSLTPROC)xsltproc $(XSLTOPTS) -o $@+ $(XSLT) $< && \
>  	mv $@+ $@
>  
>  git.info: user-manual.texi
>  	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split -o $@ user-manual.texi
>  
>  user-manual.texi: user-manual.xml
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> -	$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
> +	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) user-manual.xml --encoding=UTF-8 --to-stdout >$@++ && \
>  	$(PERL_PATH) fix-texi.perl <$@++ >$@+ && \
>  	rm $@++ && \
>  	mv $@+ $@
>  
>  user-manual.pdf: user-manual.xml
> -	$(QUIET_DBLATEX)$(RM) $@+ $@ && \
> -	$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
> +	$(QUIET_DBLATEX)$(DBLATEX) -o $@+ $(DBLATEX_COMMON) $< && \
>  	mv $@+ $@
>  
>  gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> +	$(QUIET_DB2TEXI) \
>  	($(foreach xml,$(sort $(MAN_XML)),xsltproc -o $(xml)+ texi.xsl $(xml) && \
>  		$(DOCBOOK2X_TEXI) --encoding=UTF-8 --to-stdout $(xml)+ && \
>  		rm $(xml)+ &&) true) > $@++ && \
> @@ -434,13 +424,11 @@ gitman.info: gitman.texi
>  	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
>  
>  $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
> -	$(QUIET_DB2TEXI)$(RM) $@+ $@ && \
> -	$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
> +	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@+ && \
>  	mv $@+ $@
>  
>  howto-index.txt: howto-index.sh $(HOWTO_TXT)
> -	$(QUIET_GEN)$(RM) $@+ $@ && \
> -	'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
> +	$(QUIET_GEN)'$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(HOWTO_TXT)) >$@+ && \
>  	mv $@+ $@
>  
>  $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
> @@ -450,7 +438,7 @@ WEBDOC_DEST = /pub/software/scm/git/docs
>  
>  howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../
>  $(patsubst %.txt,%.html,$(HOWTO_TXT)): %.html : %.txt GIT-ASCIIDOCFLAGS
> -	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
> +	$(QUIET_ASCIIDOC) \
>  	sed -e '1,/^$$/d' $< | \
>  	$(TXT_TO_HTML) - >$@+ && \
>  	mv $@+ $@

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> This patch series is an attempt to cleanup the Makefile of the documentation.
>
> Since v2 only certain $(RM) commands are moved from one patch to another.

The above description is correct.  I can see that the end result is
identical to the previous one.

>  Documentation/Makefile | 77 ++++++++++++++++--------------------------
>  1 file changed, 29 insertions(+), 48 deletions(-)

Is this satisfactory to everybody (I haven't taken a look beyond
comparing the end results)?

It does not seem to play well with what brian is already doing in
'seen'; it overlaps with its changes in interesting ways.  Is this
series meant to be a superset of the topic and supersedes it?  How
are you two coordinating to avoid duplicated work and stepping on
each other's toes?

Thanks.

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Commits 50cff52f1a (When generating manpages, delete outdated targets
> > first., 2007-08-02) and f9286765b2 (Documentation/Makefile: remove
> > cmd-list.made before redirecting to it., 2007-08-06) created these rm
> > instances for a very rare corner-case: building as root by mistake.
> >
> > It's odd to have workarounds here, but nowhere else in the Makefile--
> > which already fails in this stuation, starting from
> > Documentation/technical/.
> >
> > We gain nothing but complexity, so let's remove them.
> >
> > Comments-by: Jeff King <peff@peff.net>
> 
> This is curious.  I understand what other kinds of trailers like
> Helped-by and Suggested-by people sometimes use mean, but this one
> does not tell anything other than that this person had some comments
> on an earlier rounds, does not tell us what kind of comments they
> were and if something was done to address those comments or the
> comments were totally ignored or what.

If they were ignored they would not be part of the commit. It is
understood the comments were incorporated.

When you see 'Tested-by' do you wonder what was the result of the test?
I presume most people would understand that the test succeeded, and
that's why the trailer line was added.

I happen to have at hand a script that ranks the most common trailer
lines:

 * acked-by: 25% (1945)

Doesn't apply.

 * reviewed-by: 22% (1729)

This does apply, but I'm not confident Jeff would like me to add that.

 * helped-by: 17% (1336)
 * reported-by: 12% (960)
 * mentored-by: 5% (379)
 * suggested-by: 4% (281)
 * cc: 3% (222)
 * noticed-by: 2% (165)
 * tested-by: 2% (153)
 * improved-by: 1% (88)
 * thanks-to: 1% (65)
 * signed-off-by: 1% (50)
 * based-on-patch-by: 1% (50)
 * contributions-by: 1% (43)
 * co-authored-by: 1% (41)

I don't think any of these apply.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Felipe Contreras wrote:
> Junio C Hamano wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:

> > > Comments-by: Jeff King <peff@peff.net>
> > 
> > This is curious.  I understand what other kinds of trailers like
> > Helped-by and Suggested-by people sometimes use mean, but this one
> > does not tell anything other than that this person had some comments
> > on an earlier rounds, does not tell us what kind of comments they
> > were and if something was done to address those comments or the
> > comments were totally ignored or what.
> 
> If they were ignored they would not be part of the commit. It is
> understood the comments were incorporated.

Oh, and there are some instances already in the history:

7a9b0b802e (git-clean: add colors to interactive git-clean, 2013-06-25)
1b8fd46732 (git-clean: show items of del_list in columns, 2013-06-25)
52fd972d59 (git-completion: fix regression in zsh support, 2011-05-10)

Although one of those is mine.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> I happen to have at hand a script that ranks the most common trailer
> lines:
>
>  * acked-by: 25% (1945)
>
> Doesn't apply.
>
>  * reviewed-by: 22% (1729)
>
> This does apply, but I'm not confident Jeff would like me to add that.
>
>  * helped-by: 17% (1336)

I actually think people use this one to say "person X gave a
valuable input in the review discussion", which is exactly the case
here, and that was why I wondered you needed to invent a completely
new one.

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > I happen to have at hand a script that ranks the most common trailer
> > lines:
> >
> >  * acked-by: 25% (1945)
> >
> > Doesn't apply.
> >
> >  * reviewed-by: 22% (1729)
> >
> > This does apply, but I'm not confident Jeff would like me to add that.
> >
> >  * helped-by: 17% (1336)
> 
> I actually think people use this one to say "person X gave a
> valuable input in the review discussion", which is exactly the case
> here, and that was why I wondered you needed to invent a completely
> new one.

I think the opposite: "helped-by" encompasses virtually anything. A
person that reviews, reports, mentors, suggests, tests, provides a
patch, contributes, and co-authors... is helping.

The question is *how* he helped in this particular patch... By providing
comments.

-- 
Felipe Contreras

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> >  * helped-by: 17% (1336)
>> 
>> I actually think people use this one to say "person X gave a
>> valuable input in the review discussion", which is exactly the case
>> here, and that was why I wondered you needed to invent a completely
>> new one.
>
> I think the opposite: "helped-by" encompasses virtually anything...

But your own stats disagrees with your opinion, so don't invent a
new thing, period.

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Junio C Hamano wrote:
> >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> >> >  * helped-by: 17% (1336)
> >> 
> >> I actually think people use this one to say "person X gave a
> >> valuable input in the review discussion", which is exactly the case
> >> here, and that was why I wondered you needed to invent a completely
> >> new one.
> >
> > I think the opposite: "helped-by" encompasses virtually anything...
> 
> But your own stats disagrees with your opinion, so don't invent a
> new thing, period.

Stats don't have opinions.

You and I have had this discussion before, between the status quo:

	die ("could not find author in commit %s",
	     oid_to_hex(&commit->object.oid));

And:

	die("could not find author in commit %s",
		oid_to_hex(&commit->object.oid));

Eventually you yourself updated the documentation to explicitly state
that it's fine to not align the subsequent lines to the opening
parenthesis: f26443da04 (CodingGuidelines: on splitting a long line,
2014-05-02).

It is wrong to demand something that is not in the guidelines,
especially if later on the guidelines might include the very thing
supposedly frowned upon [*].


In this particular case the guideline is not missing, it actually sides
with me:

  You can also create your own tag or use one that's in common usage
  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".

If you want to forbid certain commit trailers--or limit the allowed
trailers to a sanctioned list--then update the guidelines first to
reflect that.


But I don't think it makes sense to do that, because commit
trailers--just like words, and hundreds of other things--follow a Zipf's
law, where the 10th most common word appears around 1/10th of the time.

If you graph the frequency of commit trailers vs. a Zipf distrubtion
with a 80:20 rule, it follows it almost perfectly [1]. That means 80% of the
commit trailers appear 20% of the time.

My script can calculate any quantile and for example found out that just
two lines--Acked-by and Reviewed-by--acound for 47% of all the lines.
They are the top 1%.

The top 10% is constituted by 16 lines, and the top 25% are 33.

The median is 1 appearance. That means of the 155 unique lines, half of
them have just 1 appearance, in fact, more than half: 100 of them.

For more details see the mail I just sent [2].

Unsurprisingly commit trailer lines follow the same pattern as income
distribution and musical bands popularity; the rich get richer.

As much as you might despise the poorest among commit trailers, they are
the majority, and they will keep being the majority.

Cheers.

[1] https://i.imgur.com/1wQgSlP.png
[2] https://lore.kernel.org/git/60ad75ac7ffca_2ae08208b@natae.notmuch/

[Footnote]

 * Granted, my patch back then did not match the new guideline
   perfectly, but also it wasn't wrong for reasons stated then.

-- 
Felipe Contreras

Re: [PATCH v3 3/5] doc: remove unnecessary rm instances

[Felipe Contreras]

Felipe Contreras wrote:
> Junio C Hamano wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> > 
> > > Junio C Hamano wrote:
> > >> Felipe Contreras <felipe.contreras@gmail.com> writes:
> > >> >  * helped-by: 17% (1336)
> > >> 
> > >> I actually think people use this one to say "person X gave a
> > >> valuable input in the review discussion", which is exactly the case
> > >> here, and that was why I wondered you needed to invent a completely
> > >> new one.
> > >
> > > I think the opposite: "helped-by" encompasses virtually anything...
> > 
> > But your own stats disagrees with your opinion, so don't invent a
> > new thing, period.
> 
> Stats don't have opinions.

Plus, here are some instances of commit trailer lines used only *once*
by big names of git.git (they are the authors).

2014-02-18: brought-to-attention-by: Junio C Hamano
2012-03-16: discussion-triggered-by: Junio C Hamano
2011-04-26: heavylifting-by: Junio C Hamano
2010-06-23: initial-fix-by: Junio C Hamano
2008-10-23: tested-on-freebsd-by: Junio C Hamano
2008-10-23: tested-on-aix-by: Junio C Hamano
2008-08-19: tested-on-mingw-by: Junio C Hamano
2008-03-23: reference: Junio C Hamano

2020-12-11: initial-patch-by: Johannes Schindelin
2020-11-19: pointed-out-by: Johannes Schindelin
2020-08-12: simplified-testcase-and-bisection-by: Elijah Newren
2020-04-27: documentation-by: Elijah Newren
2019-10-10: inspired-by-a-patch-by: Johannes Schindelin
2018-07-16: explanation-by: Stefan Beller
2018-01-04: also-reported-by: Johannes Schindelin
2017-09-23: reported-at: Jeff King
2017-01-06: test-provided-by: Stefan Beller
2016-10-07: issue-found-by: Jakub Narębski
2016-09-04: benchmarked-by: Christian Couder
2016-05-11: initial-test-by: Johannes Schindelin
2016-05-03: fix-proposed-by: Stefan Beller
2016-01-13: assisted-by: Johannes Schindelin
2016-01-12: derived-from-code-by: Johannes Schindelin
2015-09-24: history-dug-by: Jeff King
2015-09-09: reported-and-suggested-by: Johannes Schindelin
2015-06-30: clever-idea-by: Jeff King
2015-05-20: test-adapted-from: Jeff King
2015-03-03: edited-by: Stefan Beller
2014-12-12: tweaked-by: Michael Haggerty
2014-12-08: analyzed-by: Nguyễn Thái Ngọc Duy
2014-11-02: adapted-by: Nguyễn Thái Ngọc Duy
2014-06-08: reminded-by: Nguyễn Thái Ngọc Duy
2013-12-28: clarified-by: Jeff King
2013-08-21: revised-by: Shawn O. Pearce
2013-08-16: reported-and-analyzed-by: Nguyễn Thái Ngọc Duy
2013-07-09: more-spots-found-by: Jeff King
2013-02-23: brown-paper-bag-fixes-by: Jeff King
2012-10-12: initial-work-by: Nguyễn Thái Ngọc Duy
2012-07-22: reported-tested-and-explained-by: Jonathan Nieder
2011-09-22: tracked-down-by: Nguyễn Thái Ngọc Duy
2011-09-16: originally-by: Jakub Narębski
2011-04-28: inspired-by-patch-by: Jakub Narębski
2011-04-28: inspired-by-code-by: Jakub Narębski
2011-04-28: based-on-code-by: Jakub Narębski
2011-01-06: works-around: Jonathan Nieder
2011-01-02: helpd-by: Jonathan Nieder
2009-12-07: looks-fine-to-me-by: Jeff King
2009-11-11: originally-submitted-by: Jonathan Nieder
2009-05-15: looks-right-to-me-by: Johannes Schindelin
2008-12-26: patch-by: Shawn O. Pearce
2008-12-07: kind-of-noticed-by: Jakub Narębski
2008-07-03: backtraced-by: Shawn O. Pearce
2008-06-13: deemed-obviously-correct-by: Shawn O. Pearce
2008-03-29: patch-submitted-by: Eric Wong
2007-12-07: test-proposed-by: Jakub Narębski
2007-11-17: completely-acked-by: Jeff King
2007-09-12: proposed-by: Shawn O. Pearce
2007-06-03: hopefully-signed-off-by: Johannes Schindelin
2007-05-19: confirmed-by: Eric Wong

-- 
Felipe Contreras

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Jeff King]

On Tue, May 25, 2021 at 02:51:54AM +0900, Junio C Hamano wrote:

> >  Documentation/Makefile | 77 ++++++++++++++++--------------------------
> >  1 file changed, 29 insertions(+), 48 deletions(-)
> 
> Is this satisfactory to everybody (I haven't taken a look beyond
> comparing the end results)?

Yes, all five patches look good to me.

The "comments-by" sub-thread looked unproductive to me, so I mostly
skipped it. ;) But as the person whose name was in the trailer, I will
say that I do not care either way if it is included. It did not seem to
add anything to me. Some trailers are good for pointing later readers to
folks who may be able to help understand or debug an old commit. Some
are good for just giving credit to people who helped. "Comments-by"
didn't really seem to do either to me (and I don't feel like I
contributed much worthy of credit anyway).

-Peff

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Felipe Contreras]

Jeff King wrote:
> On Tue, May 25, 2021 at 02:51:54AM +0900, Junio C Hamano wrote:
> 
> > >  Documentation/Makefile | 77 ++++++++++++++++--------------------------
> > >  1 file changed, 29 insertions(+), 48 deletions(-)
> > 
> > Is this satisfactory to everybody (I haven't taken a look beyond
> > comparing the end results)?
> 
> Yes, all five patches look good to me.

So... Acked-by?

> The "comments-by" sub-thread looked unproductive to me, so I mostly
> skipped it. ;) But as the person whose name was in the trailer, I will
> say that I do not care either way if it is included. It did not seem to
> add anything to me. Some trailers are good for pointing later readers to
> folks who may be able to help understand or debug an old commit. Some
> are good for just giving credit to people who helped. "Comments-by"
> didn't really seem to do either to me (and I don't feel like I
> contributed much worthy of credit anyway).

But you did provide comments, and those comments resulted in changes, so
the comments-by line is accurate. And for what it's worth that commit
trailer line has been used 3 times already, which places it in the 3rd
top decile.

Perhaps some people would prefer feedback-by, but that that line has
been used *zero* times. Even brown-paper-bag-fixes-by has been used more.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Jeff King]

On Thu, May 27, 2021 at 11:59:24AM -0500, Felipe Contreras wrote:

> Jeff King wrote:
> > On Tue, May 25, 2021 at 02:51:54AM +0900, Junio C Hamano wrote:
> > 
> > > >  Documentation/Makefile | 77 ++++++++++++++++--------------------------
> > > >  1 file changed, 29 insertions(+), 48 deletions(-)
> > > 
> > > Is this satisfactory to everybody (I haven't taken a look beyond
> > > comparing the end results)?
> > 
> > Yes, all five patches look good to me.
> 
> So... Acked-by?

If you want. I have never found that trailer to be particularly helpful.

> > The "comments-by" sub-thread looked unproductive to me, so I mostly
> > skipped it. ;) But as the person whose name was in the trailer, I will
> > say that I do not care either way if it is included. It did not seem to
> > add anything to me. Some trailers are good for pointing later readers to
> > folks who may be able to help understand or debug an old commit. Some
> > are good for just giving credit to people who helped. "Comments-by"
> > didn't really seem to do either to me (and I don't feel like I
> > contributed much worthy of credit anyway).
> 
> But you did provide comments, and those comments resulted in changes, so
> the comments-by line is accurate. And for what it's worth that commit
> trailer line has been used 3 times already, which places it in the 3rd
> top decile.

I didn't say it was inaccurate. I just think it is not useful.

-Peff

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Felipe Contreras]

Jeff King wrote:
> > > The "comments-by" sub-thread looked unproductive to me, so I mostly
> > > skipped it. ;) But as the person whose name was in the trailer, I will
> > > say that I do not care either way if it is included. It did not seem to
> > > add anything to me. Some trailers are good for pointing later readers to
> > > folks who may be able to help understand or debug an old commit. Some
> > > are good for just giving credit to people who helped. "Comments-by"
> > > didn't really seem to do either to me (and I don't feel like I
> > > contributed much worthy of credit anyway).
> > 
> > But you did provide comments, and those comments resulted in changes, so
> > the comments-by line is accurate. And for what it's worth that commit
> > trailer line has been used 3 times already, which places it in the 3rd
> > top decile.
> 
> I didn't say it was inaccurate. I just think it is not useful.

I understood. I'm not saying the tag must be there because it's useful
(I agree it's usefulness is marginal at best), I'm saying including it
does not hurt.

-- 
Felipe Contreras

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, May 25, 2021 at 02:51:54AM +0900, Junio C Hamano wrote:
>
>> >  Documentation/Makefile | 77 ++++++++++++++++--------------------------
>> >  1 file changed, 29 insertions(+), 48 deletions(-)
>> 
>> Is this satisfactory to everybody (I haven't taken a look beyond
>> comparing the end results)?
>
> Yes, all five patches look good to me.

The issue that it interacts in a funny way with what brian already
has in 'seen' aside, the patches look good to me, too, so let's
start merging it down.

> The "comments-by" sub-thread looked unproductive to me, so I mostly
> skipped it. ;) But as the person whose name was in the trailer, I will
> say that I do not care either way if it is included. It did not seem to
> add anything to me. Some trailers are good for pointing later readers to
> folks who may be able to help understand or debug an old commit. Some
> are good for just giving credit to people who helped. "Comments-by"
> didn't really seem to do either to me (and I don't feel like I
> contributed much worthy of credit anyway).

Exactly the same feeling.  It didn't add any value that was why I
saw no point in using something almost nobody uses (instead of the
more commonly used Helped-by, for example).

Thanks.

Re: [PATCH v3 0/5] doc: asciidoc cleanups

[Felipe Contreras]

Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:

> > The "comments-by" sub-thread looked unproductive to me, so I mostly
> > skipped it. ;) But as the person whose name was in the trailer, I will
> > say that I do not care either way if it is included. It did not seem to
> > add anything to me. Some trailers are good for pointing later readers to
> > folks who may be able to help understand or debug an old commit. Some
> > are good for just giving credit to people who helped. "Comments-by"
> > didn't really seem to do either to me (and I don't feel like I
> > contributed much worthy of credit anyway).
> 
> Exactly the same feeling.  It didn't add any value that was why I
> saw no point in using something almost nobody uses (instead of the
> more commonly used Helped-by, for example).

As I demonstrated with probabilistic certainty; everyone uses uncommon
lines (including you). The fact that a line is uncommon doesn't
necessarily mean it's bad, in fact, I do see value in going outside the
box (like one of my favorites: Brown-paper-bag-fixes-by [4981fe750b]).

And of course if you find Comments-by so egregious, you can amend my
commit message (as you have done many times in the past). But this might
be a good opportunity to exercise the advice Jeff gave in another
thread, and even if you are not 100% happy with something, let it slide.

Cheers.

-- 
Felipe Contreras