From: Thomas Gummerer <firstname.lastname@example.org>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Junio C Hamano" <email@example.com>,
firstname.lastname@example.org, "Joel Teichroeb" <email@example.com>,
"Jeff King" <firstname.lastname@example.org>,
"Martin Ågren" <email@example.com>
Subject: Re: [PATCH v3 1/3] factor out refresh_and_write_cache function
Date: Wed, 11 Sep 2019 18:52:01 +0100 [thread overview]
Message-ID: <20190911175201.GA11444@cat> (raw)
On 09/11, Johannes Schindelin wrote:
> Hi Thomas,
> On Fri, 6 Sep 2019, Thomas Gummerer wrote:
> > Oops, I didn't realize there was another series in flight that also
> > introduces 'repo_refresh_and_write_index'. Probably should have done
> > a test merge of this with pu.
> Yep, our patches clash. I would not mind placing my patch series on top
> of yours, provided that you can make a few changes that I need ;-)
Sounds good. Looking ahead further I don't mind these changes at all!
> > Right, and if gentle is set to false, it avoids writing the index,
> > which seems fine from my perspective.
> This also suggests that it would make sense to avoid
> `LOCK_DIE_ON_ERROR`, _in particular_ because this is supposed to be a
> library function, not just a helper function for a one-shot built-in
> (don't you like how this idea "it is okay to use exit() to clean up
> after us, we don't care" comes back to bite us?).
Yup, returning an error for this definitely makes sense, especially
for future proofing.
> > > - This version allows to pass pathspec, seen and header_msg, while
> > > the one in builtin-add-i cannot limit the part of the index
> > > getting refreshed with pathspec. It wouldn't be a brain surgery
> > > to use this version and adjust the caller (there only is one) in
> > > the builtin-add-i topic.
> > 'pathspec', 'seen' and 'header_msg' are not used in my version either,
> > I just implemented it for completeness and compatibility. So I'd be
> > fine to do without them.
> Oh, why not keep them? I'd rather keep them and adjust the caller in
Great, I'm happy to keep them.
> > There's two more differences between the versions:
> > - The version in my series allows passing in write_flags to be passed
> > to write_locked_index, which is required to convert the callers in
> > builtin/merge.c.
> I can always pass in 0 as `write_flags`.
> > - Dscho's version also calls 'repo_read_index_preload()', which I
> > don't do in mine. Some callers don't need to do that, so I think it
> > would be nice to keep that outside of the
> > 'repo_refresh_and_write_index()' function.
> > I can think of a few ways forward here:
> > - I incorporate features that are needed for the builtin-add-i series
> > here, and that is rebased on top of this series.
> I'd prefer this way forward. The `builtin-add-i` patch series is
> evolving more slowly than yours.
Great! I'll send an updated version of my series soon. Thanks!
next prev parent reply other threads:[~2019-09-11 17:52 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-27 10:14 [PATCH 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-27 10:14 ` [PATCH 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-28 15:49 ` Martin Ågren
2019-08-29 17:59 ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-28 15:52 ` Martin Ågren
2019-08-29 18:00 ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-08-29 18:27 ` [PATCH v2 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-29 18:27 ` [PATCH v2 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-30 15:07 ` Martin Ågren
2019-08-30 17:06 ` Junio C Hamano
2019-09-02 17:15 ` Thomas Gummerer
2019-09-03 17:43 ` Junio C Hamano
2019-08-29 18:27 ` [PATCH v2 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-29 18:27 ` [PATCH v2 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-03 19:10 ` [PATCH v3 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-09-03 19:10 ` [PATCH v3 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-05 22:00 ` Junio C Hamano
2019-09-06 14:18 ` Thomas Gummerer
2019-09-11 10:57 ` Johannes Schindelin
2019-09-11 17:52 ` Thomas Gummerer [this message]
2019-09-12 16:46 ` Junio C Hamano
2019-09-03 19:10 ` [PATCH v3 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-03 19:10 ` [PATCH v3 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-11 18:20 ` [PATCH v4 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-09-11 18:20 ` [PATCH v4 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-11 18:20 ` [PATCH v4 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-11 18:20 ` [PATCH v4 3/3] stash: make sure to write refreshed cache Thomas Gummerer
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:
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 \
* 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
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).