| Message ID | 20210521222941.525901-1-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | doc: asciidoc cleanups | expand |
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.
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
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.
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
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.
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.
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.
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