From: Josh Steadmon <steadmon@google.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: Taylor Blau <me@ttaylorr.com>, Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, calvinwan@google.com, glencbz@gmail.com
Subject: Re: [PATCH v3 0/5] config-parse: create config parsing library
Date: Wed, 25 Oct 2023 12:37:51 -0700 [thread overview]
Message-ID: <ZTluW0Ha2W4NYaJd@google.com> (raw)
In-Reply-To: <20231024225005.1191555-1-jonathantanmy@google.com>
On 2023.10.24 15:50, Jonathan Tan wrote:
> Taylor Blau <me@ttaylorr.com> writes:
> > But I am not sure that I agree that this series is moving us in the
> > right direction necessarily. Or at least I am not convinced that
> > shipping the intermediate state is worth doing before we have callers
> > that could drop '#include "config.h"' for just the parser.
> >
> > This feels like churn that does not yield a tangible pay-off, at least
> > in the sense that the refactoring and code movement delivers us
> > something that we can substantively use today.
> >
> > I dunno.
> >
> > Thanks,
> > Taylor
>
> Thanks for calling this out. We do want our changes to be good for both
> the libification and the non-libification cases as much as possible. As
> it is, I do agree that since we won't have callers that can use the new
> parser header (I think the likeliest cause of having such a caller is
> if we have a "interpret-config" command, like "interpret-trailers"), we
> probably shouldn't merge this (at least, the last 2 patches).
>
> I think patches 1-3 are still usable (they make some internals of config
> parsing less confusing) but I'm also OK if we hold off on them until
> we find a compelling use case that motivates refactoring on the config
> parser.
Thanks everyone for the revived discussion here. I think I agree, this
series is not going in the right direction. Additionally, our internal
use case for this change has evaporated, so let's just drop the series.
We can pick it up again later if interest returns.
next prev parent reply other threads:[~2023-10-25 19:38 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-20 22:17 [PATCH 0/2] config-parse: create config parsing library Glen Choo via GitGitGadget
2023-07-20 22:17 ` [PATCH 1/2] config: return positive from git_config_parse_key() Glen Choo via GitGitGadget
2023-07-20 23:44 ` Jonathan Tan
2023-07-21 4:32 ` Junio C Hamano
2023-07-21 16:12 ` Glen Choo
2023-07-21 16:36 ` Junio C Hamano
2023-07-20 22:17 ` [PATCH 2/2] config-parse: split library out of config.[c|h] Glen Choo via GitGitGadget
2023-07-21 0:31 ` Jonathan Tan
2023-07-21 15:55 ` Glen Choo
2023-07-31 23:46 ` [RFC PATCH v1.5 0/5] config-parse: create config parsing library Glen Choo
2023-07-31 23:46 ` [RFC PATCH v1.5 1/5] config: return positive from git_config_parse_key() Glen Choo
2023-07-31 23:46 ` [RFC PATCH v1.5 2/5] config: split out config_parse_options Glen Choo
2023-07-31 23:46 ` [RFC PATCH v1.5 3/5] config: report config parse errors using cb Glen Choo
2023-08-04 21:34 ` Jonathan Tan
2023-07-31 23:46 ` [RFC PATCH v1.5 4/5] config.c: accept config_parse_options in git_config_from_stdin Glen Choo
2023-07-31 23:46 ` [RFC PATCH v1.5 5/5] config-parse: split library out of config.[c|h] Glen Choo
2023-08-23 21:53 ` [PATCH v2 0/4] config-parse: create config parsing library Josh Steadmon
2023-08-23 21:53 ` [PATCH v2 1/4] config: split out config_parse_options Josh Steadmon
2023-08-23 23:26 ` Junio C Hamano
2023-09-21 21:08 ` Josh Steadmon
2023-08-23 21:53 ` [PATCH v2 2/4] config: report config parse errors using cb Josh Steadmon
2023-08-24 1:19 ` Junio C Hamano
2023-08-24 17:31 ` Jonathan Tan
2023-08-24 18:48 ` Junio C Hamano
2023-09-21 21:11 ` Josh Steadmon
2023-09-21 23:36 ` Junio C Hamano
2023-08-23 21:53 ` [PATCH v2 3/4] config.c: accept config_parse_options in git_config_from_stdin Josh Steadmon
2023-08-23 21:53 ` [PATCH v2 4/4] config-parse: split library out of config.[c|h] Josh Steadmon
2023-08-24 20:10 ` [PATCH v2 0/4] config-parse: create config parsing library Josh Steadmon
2023-09-21 21:17 ` [PATCH v3 0/5] " Josh Steadmon
2023-09-21 21:17 ` [PATCH v3 1/5] config: split out config_parse_options Josh Steadmon
2023-10-23 17:52 ` Jonathan Tan
2023-10-23 18:46 ` Taylor Blau
2023-09-21 21:17 ` [PATCH v3 2/5] config: split do_event() into start and flush operations Josh Steadmon
2023-10-23 18:05 ` Jonathan Tan
2023-09-21 21:17 ` [PATCH v3 3/5] config: report config parse errors using cb Josh Steadmon
2023-10-23 18:41 ` Jonathan Tan
2023-10-23 19:29 ` Taylor Blau
2023-10-23 20:11 ` Junio C Hamano
2023-09-21 21:17 ` [PATCH v3 4/5] config.c: accept config_parse_options in git_config_from_stdin Josh Steadmon
2023-10-23 18:52 ` Jonathan Tan
2023-09-21 21:17 ` [PATCH v3 5/5] config-parse: split library out of config.[c|h] Josh Steadmon
2023-10-23 18:53 ` Jonathan Tan
2023-10-17 17:13 ` [PATCH v3 0/5] config-parse: create config parsing library Junio C Hamano
2023-10-23 19:34 ` Taylor Blau
2023-10-23 20:13 ` Junio C Hamano
2023-10-24 22:50 ` Jonathan Tan
2023-10-25 19:37 ` Josh Steadmon [this message]
2023-10-27 13:04 ` Junio C Hamano
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=ZTluW0Ha2W4NYaJd@google.com \
--to=steadmon@google.com \
--cc=calvinwan@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=glencbz@gmail.com \
--cc=jonathantanmy@google.com \
--cc=me@ttaylorr.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).