From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: Bug: rebase -i creates committer time inversions on 'reword'
Date: Mon, 16 Apr 2018 07:56:52 +0200 [thread overview]
Message-ID: <06c5bd54-f1b0-7fe5-6aa8-870e0ae4487d@kdbg.org> (raw)
In-Reply-To: <xmqq7ep817bq.fsf@gitster-ct.c.googlers.com>
Am 15.04.2018 um 23:35 schrieb Junio C Hamano:
> Ah, do you mean we have an internal sequence like this, when "rebase
> --continue" wants to conclude an edit/reword?
Yes, it's only 'reword' that is affected, because then subsequent picks
are processed by the original process.
> - we figure out the committer ident, which grabs a timestamp and
> cache it;
>
> - we spawn "commit" to conclude the stopped step, letting it record
> its beginning time (which is a bit older than the above) or its
> ending time (which is much older due to human typing speed);
Younger in both cases, of course. According to my tests, we seem to pick
the beginning time, because the first 'reword'ed commit typically has
the same timestamp as the preceding picks. Later 'reword'ed commits have
noticably younger timestamps.
> - subsequent "picks" are made in the same process, and share the
> timestamp we grabbed in the first step, which is older than the
> second one.
>
> I guess we'd want a mechanism to tell ident.c layer "discard the
> cached one, as we are no longer in the same automated sequence", and
> use that whenever we spawn an editor (or otherwise go interactive).
Frankly, I think that this caching is overengineered (or prematurly
optimized). If the design requires that different callers of datestamp()
must see the same time, then the design is broken. In a fixed design,
there would be a single call of datestamp() in advance, and then the
timestamp, which then obviously is a very important piece of data, would
be passed along as required.
-- Hannes
next prev parent reply other threads:[~2018-04-16 5:56 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-13 16:52 Bug: rebase -i creates committer time inversions on 'reword' Johannes Sixt
2018-04-14 11:15 ` Phillip Wood
2018-04-14 13:11 ` Johannes Schindelin
2018-04-16 9:48 ` Phillip Wood
2018-04-19 9:17 ` Phillip Wood
2018-04-15 21:35 ` Junio C Hamano
2018-04-16 5:56 ` Johannes Sixt [this message]
2018-04-17 1:37 ` Junio C Hamano
2018-04-18 10:19 ` Phillip Wood
2018-04-18 10:22 ` [RFC PATCH] ident: don't cache default date Phillip Wood
2018-04-18 11:27 ` Ævar Arnfjörð Bjarmason
2018-04-18 17:47 ` Phillip Wood
2018-04-18 18:15 ` Johannes Sixt
2018-04-18 21:15 ` Junio C Hamano
2018-04-19 9:15 ` Phillip Wood
2018-04-20 8:11 ` Johannes Schindelin
2018-04-20 9:41 ` Phillip Wood
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=06c5bd54-f1b0-7fe5-6aa8-870e0ae4487d@kdbg.org \
--to=j6t@kdbg.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
/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).