Elijah Newren <newren@xxxxxxxxx> writes:
> Subject: Re: [PATCH 1/4] sequencer: Warn when internal merge may be suboptimal due to renameLimit
Style: please downcase s/Warn/warn/ to fit this better in "shortlog --no-merges"
output.
> Having renamed files silently treated as deleted/modified conflicts
> instead of correctly resolving the renamed/modified case has caused lots
> of pain for some users. Eventually, some of them figured out to set
> merge.renameLimit to something higher, but without feedback about what
> value it should have, they were just repeatedly guessing and retrying.
It would have been _much_ easier to read if you refrained from
stating the gripes more prominently than the source of the gripe
that you are fixing. E.g.
If one side of a merge have renamed more paths than
merge.renamelimit since the sides diverged, the recursive
merge strategy stops detecting renames and instead leaves
these many paths as delete/modify conflicts, without warning
what is going on and giving hints on how to tell Git that it
is allowed to spend more cycles to detect renames.
perhaps.
The addition of a call to diff_warn_rename_limit looks quite
sensible.
Thanks.
> Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> ---
> sequencer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index f2a10cc4f2..2b4cecb617 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -462,6 +462,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> if (is_rebase_i(opts) && clean <= 0)
> fputs(o.obuf.buf, stdout);
> strbuf_release(&o.obuf);
> + diff_warn_rename_limit("merge.renamelimit", o.needed_rename_limit, 0);
> if (clean < 0)
> return clean;
