git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael J Gruber <git@grubix.eu>
Cc: git@vger.kernel.org, Ekelhart Jakob <jakob.ekelhart@fsw.at>,
	Jeff King <peff@peff.net>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 2/3] merge-base: return fork-point outside reflog
Date: Tue, 03 Oct 2017 15:05:43 +0900	[thread overview]
Message-ID: <xmqq7ewckbpk.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqa81njds0.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Fri, 22 Sep 2017 18:14:39 +0900")

Junio C Hamano <gitster@pobox.com> writes:

> Michael J Gruber <git@grubix.eu> writes:
>
>> I'm still trying to understand what the original intent was: If we
>> abstract from the implementation (as we should, as you rightly
>> emphasize) and talk about historical tips then we have to ask ourselves:
>> - What is "historical"?
>> - What is tip?
>> - Tip of what, i.e. what is a "branch"?
>
> The feature was meant to be a solution for "upstream rebased the
> branch I based my work on."
> ...

So, what is the status of this thing?

While I think 1/3 and 3/3 of these three definitely make sense, I do
not think "fork-point outside reflog" does as-is If it is not even
part of the commits that were known to be at the tip some time in
the past (including "right now"---which is the fix you made with 3/3
is about), then the patch may make the command return something in
more situations, and these extra things that it returns might even
be improvements, but they are definitely not "fork-points".

To be quite honest, I am not convinced that the extra output you
would get out of the command by removing the latter half of "which
are the ancestors that were known to be at the tip?" would always
give better commit to use as the beginning of the topic to be
rebased, as I do not see any reasoning behind why, unlike the
filtered case where there _is_ a strong reasoning (with explained
limitation) behind it.

As long as the code misidentifies and picks a commits deeper than
necessary, which will force the user to say "rebase --skip", I think
we are OK.  What we want to absolutely avoid is the opposite;
somehow the code misidentifies a commit that is part of the work you
want to rebase as a recommended fork-point, which would cause the
rebase to silently lose changes.  I do not think I saw why it won't
happen explained in the log message of 2/3 at all.

  reply	other threads:[~2017-10-03  6:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <c76e76a4ef11480da9995b0bec5a70e1@SFSWW2K12EX02.intern.fsw.at>
2017-09-13 15:07 ` merge-base not working as expected when base is ahead Ekelhart Jakob
2017-09-14  8:09   ` Michael J Gruber
2017-09-14 13:15     ` [PATCH 0/3] merge-base --fork-point fixes Michael J Gruber
2017-09-14 13:15       ` [PATCH 1/3] t6010: test actual test output Michael J Gruber
2017-09-14 14:34         ` Jeff King
2017-09-15 10:01           ` Michael J Gruber
2017-09-15 11:20             ` Jeff King
2017-09-14 13:15       ` [PATCH 2/3] merge-base: return fork-point outside reflog Michael J Gruber
2017-09-15  2:48         ` Junio C Hamano
2017-09-15  9:59           ` Phillip Wood
2017-09-15 10:23           ` Michael J Gruber
2017-09-15 18:24             ` Junio C Hamano
2017-09-21  6:27               ` Junio C Hamano
2017-09-21  9:39                 ` Michael J Gruber
2017-09-22  1:49                   ` Junio C Hamano
2017-09-22  8:34                     ` Michael J Gruber
2017-09-22  9:14                       ` Junio C Hamano
2017-10-03  6:05                         ` Junio C Hamano [this message]
2017-11-08  8:52                           ` Ekelhart Jakob
2017-11-08  9:33                             ` Michael J Gruber
2017-11-09  2:49                               ` Junio C Hamano
2017-09-14 13:15       ` [PATCH 3/3] merge-base: find fork-point outside partial reflog Michael J Gruber
2017-09-14 14:37         ` Jeff King
2017-09-14 13:49       ` [PATCH 0/3] merge-base --fork-point fixes Johannes Schindelin
2017-09-14 14:38       ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7ewckbpk.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@grubix.eu \
    --cc=git@vger.kernel.org \
    --cc=jakob.ekelhart@fsw.at \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).