From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Introduce a hook to run after formatting patches
Date: Mon, 17 Nov 2014 11:06:12 -0800 [thread overview]
Message-ID: <xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqzjbryonp.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Sun, 16 Nov 2014 10:40:42 -0800")
Junio C Hamano <gitster@pobox•com> writes:
> Stefan Beller <sbeller@google•com> writes:
>
>> +post-format-patch
>> +~~~~~~~~~~~~
>> +
>> +This hook is called after format-patch created a patch and it is
>> +invoked with the filename of the patch as the first parameter.
>
> Such an interface would not work well with --stdout mode, would it?
>
> And if this only works with output generated into the files, then
>
> $ git format-patch $range | xargs -n1 $your_post_processing_script
>
> would do the same without any change to Git, I would imagine.
>
> So I would have to say that I am fairly negative on this change in
> the presented form.
>
> An alternative design to implement this as a post-processing filter
> to work for both "to individual files" and "to standard output
> stream" output filter may be possible, but even in that case I am
> not sure if it is worth the churn.
>
> In general I'd look at post-anything hook that works locally with a
> great suspicion, so that may partly be where my comment above is
> coming from. I dunno.
Another reason, in addition to that this only works on the already
created output files, why I find this particular design distasteful
(I am not saying that there should be an easy way to drop cruft left
by third-party systems such as "Change-id:" line) is because the
mechanism the patch adds does not attempt to take advantage of being
inside Git, so the "xargs -n1" above is strictly an equivalent. You
have a chance to make the life better for users, but not you are not
doing so.
The design of this feature could be made to allow the user to
specify a filter to munge _ONLY_ the log message part. For example,
just after logmsg_reencode() returns the proposed commit log message
to msg in pretty.c::pretty_print_commit(), you can detect a request
to use some external filter program and let the program munge the
message. With such a design:
* The external filter your users would write does not have to worry
about limiting its damage only to the log message part, as it
will never see the patch text part; and
* The same mechanism would work just as well for --stdout mode.
The former is what I mean by "to take advantage of being inside".
Incidentally, it falls into #2 of "5 valid reasons to admit a new
hook" [*1*].
[Reference]
*1* http://thread.gmane.org/gmane.comp.version-control.git/232809/focus=71069
next prev parent reply other threads:[~2014-11-17 19:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-15 0:47 [PATCH] Introduce a hook to run after formatting patches Stefan Beller
2014-11-15 10:44 ` Philip Oakley
2014-11-16 18:40 ` Junio C Hamano
2014-11-17 19:01 ` Stefan Beller
2014-11-17 19:06 ` Junio C Hamano [this message]
2014-11-17 19:20 ` Junio C Hamano
2014-11-18 6:40 ` Christian Couder
2014-11-20 23:26 ` Stefan Beller
2014-11-20 23:33 ` Junio C Hamano
2014-11-21 4:31 ` Christian Couder
2014-11-18 2:30 ` Stefan Beller
2014-11-18 17:02 ` Junio C Hamano
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=xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox