{"thread":{"id":"49247","subject":"[PATCH] doc: Don't echo sed command for manpage-base-url.xsl","startedAt":"2018-08-28T21:21:24Z","lastAt":"2018-08-29T22:37:12Z","messageCount":9,"participants":["Tim Schumacher","Eric Sunshine","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"356750","messageId":"20180828212104.2515-1-timschumi@gmx.de","threadId":"49247","inReplyTo":null,"subject":"[PATCH] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Tim Schumacher","fromEmail":"timschumi@gmx.de","sentAt":"2018-08-28T21:21:04Z","receivedAt":"2018-08-28T21:21:24Z","isPatch":true,"sender":{"key":"timschumi@gmx.de","avatar":"https://avatars.githubusercontent.com/u/16820960?v=4"},"body":"Previously, the sed command for generating manpage-base-url.xsl\nwas printed to the console when being run.\n\nFor the purpose of silencing it, define a $(QUIET) variable which\ncontains an '@' if verbose mode isn't enabled and which is empty\notherwise. This just silences the command invocation without doing\nanything else.\n\nSigned-off-by: Tim Schumacher \n---\n Documentation/Makefile | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a42dcfc74..45454e9b5 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -217,6 +217,7 @@ endif\n \n ifneq ($(findstring $(MAKEFLAGS),s),s)\n ifndef V\n+\tQUIET\t\t= @\n \tQUIET_ASCIIDOC\t= @echo ' ' ASCIIDOC $@;\n \tQUIET_XMLTO\t= @echo ' ' XMLTO $@;\n \tQUIET_DB2TEXI\t= @echo ' ' DB2TEXI $@;\n@@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf\n \tmv $@+ $@\n \n manpage-base-url.xsl: manpage-base-url.xsl.in\n-\tsed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n+\t$(QUIET)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n %.1 %.5 %.7 : %.xml manpage-base-url.xsl\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n-- \n2.19.0.rc0\n\n"},{"id":"356763","messageId":"CAPig+cRnPziVKe0pxqb+QTMTZCDvWvLfQUFLU53aYuOW4Mm52A@mail.gmail.com","threadId":"49247","inReplyTo":"20180828212104.2515-1-timschumi@gmx.de","subject":"Re: [PATCH] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T21:25:42Z","receivedAt":"2018-08-28T21:25:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 28, 2018 at 5:21 PM Tim Schumacher wrote:\n> Previously, the sed command for generating manpage-base-url.xsl\n> was printed to the console when being run.\n>\n> For the purpose of silencing it, define a $(QUIET) variable which\n> contains an '@' if verbose mode isn't enabled and which is empty\n> otherwise. This just silences the command invocation without doing\n> anything else.\n\nOne thing that is missing from this commit message is the explanation\nof _why_ this is desirable. And, why only this command? Without\nunderstanding the underlying reason(s), it's hard to judge the value\nof this change.\n\nThanks.\n"},{"id":"356769","messageId":"xmqqsh2ydvj4.fsf@gitster-ct.c.googlers.com","threadId":"49247","inReplyTo":"20180828212104.2515-1-timschumi@gmx.de","subject":"Re: [PATCH] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-28T21:35:27Z","receivedAt":"2018-08-28T21:35:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Schumacher writes:\n\n> Previously, the sed command for generating manpage-base-url.xsl\n> was printed to the console when being run.\n>\n> For the purpose of silencing it, define a $(QUIET) variable which\n> contains an '@' if verbose mode isn't enabled and which is empty\n> otherwise. This just silences the command invocation without doing\n> anything else.\n>\n> Signed-off-by: Tim Schumacher \n> ---\n\nI am not sure if this is a good change. All these QUIET_$TOOL hide\ndetails of running the $TOOL to produce the final output of the step,\nbut they still do report what they are creating via which $TOOL.\n\nShouldn't the step to create manpage-base-url.xsl be the same? The\ndetail of creating it (i.e. token @@MAN_BASE_URL@@ is replaced with\nthe actual value) may want to be squelched, but shouldn't we still\nbe reporting that we are creating manpage-base-url.xsl file?\n\n> Documentation/Makefile | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index a42dcfc74..45454e9b5 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -217,6 +217,7 @@ endif\n> \n> ifneq ($(findstring $(MAKEFLAGS),s),s)\n> ifndef V\n> +\tQUIET\t\t= @\n> \tQUIET_ASCIIDOC\t= @echo ' ' ASCIIDOC $@;\n> \tQUIET_XMLTO\t= @echo ' ' XMLTO $@;\n> \tQUIET_DB2TEXI\t= @echo ' ' DB2TEXI $@;\n> @@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf\n> \tmv $@+ $@\n> \n> manpage-base-url.xsl: manpage-base-url.xsl.in\n> -\tsed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n> +\t$(QUIET)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n> \n> %.1 %.5 %.7 : %.xml manpage-base-url.xsl\n> \t$(QUIET_XMLTO)$(RM) $@ && \\\n"},{"id":"356837","messageId":"20180829134334.14619-1-timschumi@gmx.de","threadId":"49247","inReplyTo":"20180828212104.2515-1-timschumi@gmx.de","subject":"[PATCH v2] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Tim Schumacher","fromEmail":"timschumi@gmx.de","sentAt":"2018-08-29T13:43:34Z","receivedAt":"2018-08-29T13:43:56Z","isPatch":true,"sender":{"key":"timschumi@gmx.de","avatar":"https://avatars.githubusercontent.com/u/16820960?v=4"},"body":"Previously, the sed command for generating manpage-base-url.xsl\nwas printed to the console when being run.\n\nMake the console output for this rule similiar to all the\nother rules by printing a short status message instead of\nthe whole command.\n\nSigned-off-by: Tim Schumacher \n---\n\nTo Junio C Hamano:\nThe rule does now print \"SED manpage-base-url.xsl\"\nto the console, which is similiar to other QUIET_$TOOL\nrules.\n\nTo Eric Sunshine:\nI reworded the commit message to focus more on _why_\nthe patch is relevant.\n\n---\n\n Documentation/Makefile | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a42dcfc74..cbf33c5a7 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -225,6 +225,7 @@ ifndef V\n \tQUIET_XSLTPROC\t= @echo ' ' XSLTPROC $@;\n \tQUIET_GEN\t= @echo ' ' GEN $@;\n \tQUIET_LINT\t= @echo ' ' LINT $@;\n+\tQUIET_SED\t= @echo ' ' SED $@;\n \tQUIET_STDERR\t= 2> /dev/null\n \tQUIET_SUBDIR0\t= +@subdir=\n \tQUIET_SUBDIR1\t= ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n@@ -344,7 +345,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf\n \tmv $@+ $@\n \n manpage-base-url.xsl: manpage-base-url.xsl.in\n-\tsed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n+\t$(QUIET_SED)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n %.1 %.5 %.7 : %.xml manpage-base-url.xsl\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n-- \n2.19.0.rc1.1.g093671f86\n\n"},{"id":"356851","messageId":"20180829154720.20297-1-timschumi@gmx.de","threadId":"49247","inReplyTo":"20180829134334.14619-1-timschumi@gmx.de","subject":"[PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Tim Schumacher","fromEmail":"timschumi@gmx.de","sentAt":"2018-08-29T15:47:20Z","receivedAt":"2018-08-29T15:48:15Z","isPatch":true,"sender":{"key":"timschumi@gmx.de","avatar":"https://avatars.githubusercontent.com/u/16820960?v=4"},"body":"Previously, the sed command for generating manpage-base-url.xsl\nwas printed to the console when being run.\n\nMake the console output for this rule similiar to all the\nother rules by printing a short status message instead of\nthe whole command.\n\nSigned-off-by: Tim Schumacher \n---\n Documentation/Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a42dcfc74..95f6a321f 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -344,7 +344,7 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf\n \tmv $@+ $@\n \n manpage-base-url.xsl: manpage-base-url.xsl.in\n-\tsed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n+\t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n %.1 %.5 %.7 : %.xml manpage-base-url.xsl\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n-- \n2.19.0.rc1.1.g093671f86\n\n"},{"id":"356855","messageId":"20180829164930.GA170940@aiede.svl.corp.google.com","threadId":"49247","inReplyTo":"20180829134334.14619-1-timschumi@gmx.de","subject":"Re: [PATCH v2] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-29T16:49:30Z","receivedAt":"2018-08-29T16:49:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tim Schumacher wrote:\n\n> Subject: doc: Don't echo sed command for manpage-base-url.xsl\n\nAt first glance, I thought this was going to change the rendered\ndocumentation in some way. Maybe this should say\n\n\tMakefile: make 'sed' commands quieter\n\n? That led me to look in the Makefile, where it appears we already do\nthis for some sed commands, using QUIET_GEN. What would you think of\nusing the same for manpage-base-url.xsl?\n\nThanks,\nJonathan\n"},{"id":"356856","messageId":"20180829165540.GB170940@aiede.svl.corp.google.com","threadId":"49247","inReplyTo":"20180829154720.20297-1-timschumi@gmx.de","subject":"Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-08-29T16:55:40Z","receivedAt":"2018-08-29T16:55:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Tim Schumacher wrote:\n\n> Subject: doc: Don't echo sed command for manpage-base-url.xsl\n\nCribbing from my review of v2: a description like\n\n\tDocumentation/Makefile: make manpage-base-url.xsl generation quieter\n\nwould make it more obvious what this does when viewed in \"git log\n--oneline\".\n\n> Previously, the sed command for generating manpage-base-url.xsl\n> was printed to the console when being run.\n>\n> Make the console output for this rule similiar to all the\n> other rules by printing a short status message instead of\n> the whole command.\n>\n> Signed-off-by: Tim Schumacher \n> ---\n> Documentation/Makefile | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nOh! Ignore my reply to v2; looks like you anticipated what I was\ngoing to suggest already. For next time, if you include a note about\nwhat changed between versions after the --- delimiter, that can help\nsave some time.\n\nWith or without the suggested commit message tweak,\n\nReviewed-by: Jonathan Nieder \n\nThank you.\n"},{"id":"356861","messageId":"c38b994b-b77d-fe03-42f9-9e22ac92e98d@gmx.de","threadId":"49247","inReplyTo":"20180829165540.GB170940@aiede.svl.corp.google.com","subject":"Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Tim Schumacher","fromEmail":"timschumi@gmx.de","sentAt":"2018-08-29T17:42:24Z","receivedAt":"2018-08-29T17:42:38Z","isPatch":true,"sender":{"key":"timschumi@gmx.de","avatar":"https://avatars.githubusercontent.com/u/16820960?v=4"},"body":"On 29.08.18 18:55, Jonathan Nieder wrote:\n> Tim Schumacher wrote:\n> \n>> Subject: doc: Don't echo sed command for manpage-base-url.xsl\n> \n> Cribbing from my review of v2: a description like\n> \n> \tDocumentation/Makefile: make manpage-base-url.xsl generation quieter\n> \n> would make it more obvious what this does when viewed in \"git log\n> --oneline\".\n\nimho, the \"Documentation/Makefile\" is a bit too long (about 1/3 of the\nfirst line). Would it be advisable to keep the previous prefix of \"doc\"\n(which would shorten down the line from 68 characters down to 49) and\nto use only the second part of your proposed first line? Depending on\nthe response I would address this in v4 of this patch.\n\n> \n>> Previously, the sed command for generating manpage-base-url.xsl\n>> was printed to the console when being run.\n>>\n>> Make the console output for this rule similiar to all the\n>> other rules by printing a short status message instead of\n>> the whole command.\n>>\n>> Signed-off-by: Tim Schumacher \n>> ---\n>> Documentation/Makefile | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> Oh! Ignore my reply to v2; looks like you anticipated what I was\n> going to suggest already. For next time, if you include a note about\n> what changed between versions after the --- delimiter, that can help\n> save some time.\n> \n\nThe change to QUIET_GEN was proposed by Junio, but that E-Mail\nwasn't CC'ed to the mailing list, probably due to him typing\nthe response on a phone.\n\nI originally included a note about the change as well, but I\nforgot to copy it over to the new patch after I generated a\nsecond version of v3.\n\n> With or without the suggested commit message tweak,\n> \n> Reviewed-by: Jonathan Nieder \n> \n> Thank you.\n> \n\nThanks for the review!\n"},{"id":"356908","messageId":"xmqqtvnc94vi.fsf@gitster-ct.c.googlers.com","threadId":"49247","inReplyTo":"20180829165540.GB170940@aiede.svl.corp.google.com","subject":"Re: [PATCH v3] doc: Don't echo sed command for manpage-base-url.xsl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-29T19:19:00Z","receivedAt":"2018-08-29T22:37:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Tim Schumacher wrote:\n>\n>> Subject: doc: Don't echo sed command for manpage-base-url.xsl\n>\n> Cribbing from my review of v2: a description like\n>\n> \tDocumentation/Makefile: make manpage-base-url.xsl generation quieter\n>\n> would make it more obvious what this does when viewed in \"git log\n> --oneline\".\n\nSounds good; let's take it.\n\n>> Previously, the sed command for generating manpage-base-url.xsl\n>> was printed to the console when being run.\n\nThe convention is that we talk about the state before the current\nseries in question is applied in the present tense, so \"previously\"\nis not needed. Perhaps\n\n The exact sed command to generate manpage-base-url.xsl appears in\n the output, unlike the rules for other files that by default only\n show summary.\n\nis sufficient. The output is not always going to \"the console\", and\nit is not like we change behaviour depending on where the output is\ngoing, so it is misleading to say \"the console\" (iow, the phrase \"to\nthe console\" has negative information density in the above\nsentence).\n\n>> Make the console output for this rule similiar to all the\n>> other rules by printing a short status message instead of\n>> the whole command.\n\nLikewise, s/console //;\n\nI'll all do the above tweaks while queueing.\n\nThanks, both.\n\n>>\n>> Signed-off-by: Tim Schumacher \n>> ---\n>> Documentation/Makefile | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> Oh! Ignore my reply to v2; looks like you anticipated what I was\n> going to suggest already. For next time, if you include a note about\n> what changed between versions after the --- delimiter, that can help\n> save some time.\n>\n> With or without the suggested commit message tweak,\n>\n> Reviewed-by: Jonathan Nieder \n>\n> Thank you.\n"}]}