From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
Michael Haggerty <mhagger@alum.mit.edu>,
git@vger.kernel.org, Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [PATCH] t0090: mark add-interactive test with PERL prerequisite
Date: Tue, 18 Nov 2014 13:44:02 -0500 [thread overview]
Message-ID: <20141118184402.GA13479@peff.net> (raw)
In-Reply-To: <20141118183838.GD6527@google.com>
On Tue, Nov 18, 2014 at 10:38:38AM -0800, Jonathan Nieder wrote:
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
> > +# This makes sure we depend on the NO_PERL setting itself.
> > +$(patsubst %.perl,%,$(SCRIPT_PERL)): GIT-BUILD-OPTIONS
> > +
> > ifndef NO_PERL
> > $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
>
> Why do these repeat the 'patsubst ...' expression instead of using
> SCRIPT_PERL_GEN, by the way?
Dunno. I just cargo-culted from the context lines.
> -- >8 --
> Subject: Makefile: simplify by using SCRIPT_{PERL,SH}_GEN macros
>
> SCRIPT_PERL_GEN is defined as $(patsubst %.perl,%,$(SCRIPT_PERL)) for
> use in targets like build-perl-script used by makefiles in
> subdirectories that override SCRIPT_PERL (see v1.8.2-rc0~17^2,
> "git-remote-mediawiki: use toplevel's Makefile", 2013-02-08).
>
> The same expression is used in the rules that actually write the
> generated perl scripts, and since this rules were introduced before
> SCRIPT_PERL_GEN, they use the longhand instead of that macro. Use the
> macro to make reading easier.
>
> Likewise for SCRIPT_SH_GEN. The Python rules already got the same
> simplification in v1.8.4-rc0~162^2~8 (2013-05-24).
This makes sense, and looking over the Makefile, I don't see how it
could cause any bad side effects.
Minor nit:
s/this rules/these rules/
in your commit message. Otherwise:
Reviewed-by: Jeff King <peff@peff.net>
-Peff
next prev parent reply other threads:[~2014-11-18 18:44 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-18 17:22 [PATCH] t0090: mark add-interactive test with PERL prerequisite Jeff King
2014-11-18 17:29 ` [PATCH] t960[34]: mark cvsimport tests as requiring perl Jeff King
2014-11-18 18:56 ` Jonathan Nieder
2014-11-18 19:15 ` Jeff King
2014-11-18 17:43 ` [PATCH] t0090: mark add-interactive test with PERL prerequisite Jeff King
2014-11-18 18:38 ` Jonathan Nieder
2014-11-18 18:43 ` Jonathan Nieder
2014-11-18 18:49 ` Jeff King
2014-11-18 23:10 ` Pete Wyckoff
2014-11-18 18:44 ` Jeff King [this message]
2014-11-18 18:51 ` Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141118184402.GA13479@peff.net \
--to=peff@peff.net \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=mhagger@alum.mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.