Re: [PATCH] doc/Makefile: remove tmp-doc-diff on "make clean"

2018-08-30 Thread Jeff King
On Thu, Aug 30, 2018 at 04:34:43PM -0400, Eric Sunshine wrote:

> On Thu, Aug 30, 2018 at 3:55 PM Jeff King  wrote:
> > The tmp-doc-diff directory isn't strictly a build product of
> > the Makefile, since it's only present if you manually run
> > the doc-diff script.  But anybody running "make clean" would
> > probably want it to go away.
> >
> > Suggested-by: Eric Sunshine 
> > Signed-off-by: Jeff King 
> > ---
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > @@ -332,6 +332,7 @@ clean:
> > $(RM) manpage-base-url.xsl
> > +   $(RM) -r tmp-doc-diff
> 
> Taking into consideration that people might be surprised and alarmed
> to find "git worktree list" showing a worktree they didn't explicitly
> create, would it make sense to do something like this?
> 
> clean:
> ...
> -git worktree remove -f tmp-doc-diff 2>/dev/null
> $(RM) -r tmp-doc-diff

Seems reasonable. Again, I don't have a strong feeling. It's a little
strange to me for the Makefile to be touching bits outside of the actual
working tree. But then, creating a separate worktree in the first place
is perhaps a little weird.

I dunno. Maybe you are right that worktrees are a bad fit here.

-Peff


Re: [PATCH] doc/Makefile: remove tmp-doc-diff on "make clean"

2018-08-30 Thread Eric Sunshine
On Thu, Aug 30, 2018 at 4:34 PM Eric Sunshine  wrote:
> Taking into consideration that people might be surprised and alarmed
> to find "git worktree list" showing a worktree they didn't explicitly
> create, would it make sense to do something like this?
>
> clean:
> ...
> -git worktree remove -f tmp-doc-diff 2>/dev/null
> $(RM) -r tmp-doc-diff

More accurately:

-git worktree remove -f Documentation/tmp-doc-diff/worktree 2>/dev/null
$(RM) -r tmp-doc-diff


Re: [PATCH] doc/Makefile: remove tmp-doc-diff on "make clean"

2018-08-30 Thread Eric Sunshine
On Thu, Aug 30, 2018 at 3:55 PM Jeff King  wrote:
> The tmp-doc-diff directory isn't strictly a build product of
> the Makefile, since it's only present if you manually run
> the doc-diff script.  But anybody running "make clean" would
> probably want it to go away.
>
> Suggested-by: Eric Sunshine 
> Signed-off-by: Jeff King 
> ---
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> @@ -332,6 +332,7 @@ clean:
> $(RM) manpage-base-url.xsl
> +   $(RM) -r tmp-doc-diff

Taking into consideration that people might be surprised and alarmed
to find "git worktree list" showing a worktree they didn't explicitly
create, would it make sense to do something like this?

clean:
...
-git worktree remove -f tmp-doc-diff 2>/dev/null
$(RM) -r tmp-doc-diff


[PATCH] doc/Makefile: remove tmp-doc-diff on "make clean"

2018-08-30 Thread Jeff King
The tmp-doc-diff directory isn't strictly a build product of
the Makefile, since it's only present if you manually run
the doc-diff script.  But anybody running "make clean" would
probably want it to go away.

Suggested-by: Eric Sunshine 
Signed-off-by: Jeff King 
---
> Another fixup for jk/diff-rendered-docs,[...]

And here's one more. I don't have a strong opinion on this myself, but
it seems sensible. I doubt anybody cares overly much about the cost of
an extra $(RM) during "make clean" (and if they do, we really ought to
consider joining the existing ones into a single invocation).

 Documentation/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index a42dcfc745..99a349ce9a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -332,6 +332,7 @@ clean:
$(RM) SubmittingPatches.txt
$(RM) $(cmds_txt) $(mergetools_txt) *.made
$(RM) manpage-base-url.xsl
+   $(RM) -r tmp-doc-diff
 
 $(MAN_HTML): %.html : %.txt asciidoc.conf
$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
-- 
2.19.0.rc1.546.g3fcb3c0d7c