From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: es/format-patch-{inter,range}diff, was Re: What's cooking in git.git (Aug 2018, #06; Wed, 29)
Date: Thu, 30 Aug 2018 16:01:34 +0200 (DST) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.1808301548560.71@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <xmqqbm9kajhu.fsf@gitster-ct.c.googlers.com>
Hi Junio,
On Wed, 29 Aug 2018, Junio C Hamano wrote:
> * es/format-patch-interdiff (2018-07-23) 6 commits
> - format-patch: allow --interdiff to apply to a lone-patch
> - log-tree: show_log: make commentary block delimiting reusable
> - interdiff: teach show_interdiff() to indent interdiff
> - format-patch: teach --interdiff to respect -v/--reroll-count
> - format-patch: add --interdiff option to embed diff in cover letter
> - format-patch: allow additional generated content in make_cover_letter()
> (this branch is used by es/format-patch-rangediff.)
>
> "git format-patch" learned a new "--interdiff" option to explain
> the difference between this version and the previous atttempt in
> the cover letter (or after the tree-dashes as a comment).
>
> What's the doneness of this one?
> cf. <CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com>
I looked over the changes online, and I think they are good.
The only slightly iffy thing I found was using the function parameter
`rerolled` as printf-style format in
https://github.com/gitgitgadget/git/compare/es/format-patch-interdiff~6...es/format-patch-interdiff#diff-71d4a6bddc3e479f9abf11900878a0b2R1430:
static const char *diff_title(struct strbuf *sb, int reroll_count,
const char *generic, const char *rerolled)
{
if (reroll_count <= 0)
strbuf_addstr(sb, generic);
else /* RFC may be v0, so allow -v1 to diff against v0 */
strbuf_addf(sb, rerolled, reroll_count - 1);
return sb->buf;
}
I guess that's okay, though. (I would have done it differently, but that
would have meant playing sentence lego with the "Interdiff against v%d:"
string.)
> * es/format-patch-rangediff (2018-08-14) 10 commits
> - format-patch: allow --range-diff to apply to a lone-patch
> - format-patch: add --creation-factor tweak for --range-diff
> - format-patch: teach --range-diff to respect -v/--reroll-count
> - format-patch: extend --range-diff to accept revision range
> - format-patch: add --range-diff option to embed diff in cover letter
> - range-diff: relieve callers of low-level configuration burden
> - range-diff: publish default creation factor
> - range-diff: respect diff_option.file rather than assuming 'stdout'
> - Merge branch 'es/format-patch-interdiff' into es/format-patch-rangediff
> - Merge branch 'js/range-diff' into es/format-patch-rangediff
> (this branch uses es/format-patch-interdiff.)
>
> "git format-patch" learned a new "--range-diff" option to explain
> the difference between this version and the previous atttempt in
> the cover letter (or after the tree-dashes as a comment).
>
> What's the doneness of this one?
I just had a look at the diff online, and I think this is ready for next.
Personally, I would have put the infer_range_diff_ranges() function
(https://github.com/gitgitgadget/git/compare/es/format-patch-rangediff~8...es/format-patch-rangediff#diff-71d4a6bddc3e479f9abf11900878a0b2R1448)
into `range-diff.c`, but it is too minor a thing to ask for a new patch
series iteration.
It also looks slightly murky to me that `show_range_diff()` is now using
a copy of the `diffopts` (see
https://github.com/gitgitgadget/git/compare/es/format-patch-rangediff~8...es/format-patch-rangediff#diff-ab6f5eca48b8e84edf999acbe3fe7553R435),
but I have no idea how to do this in a more elegant manner, either.
In short: from my point of view, both topics are ready for `next`.
Ciao,
Dscho
next prev parent reply other threads:[~2018-08-30 14:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-29 22:35 What's cooking in git.git (Aug 2018, #06; Wed, 29) Junio C Hamano
2018-08-30 0:22 ` Elijah Newren
2018-08-30 15:44 ` Junio C Hamano
2018-08-30 16:11 ` Elijah Newren
2018-08-31 19:53 ` Junio C Hamano
2018-09-01 4:32 ` Kaartic Sivaraam
2018-08-30 14:01 ` Johannes Schindelin [this message]
2018-09-03 4:23 ` Stephen & Linda Smith
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=nycvar.QRO.7.76.6.1808301548560.71@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).