From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Tanay Abhra <tanayabh@gmail.com>,
git@vger.kernel.org, Ramkumar Ramachandra <artagnon@gmail.com>
Subject: Re: [PATCH v7 1/2] add `config_set` API for caching config-like files
Date: Wed, 09 Jul 2014 10:44:18 -0700 [thread overview]
Message-ID: <xmqq38ea77gt.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqmwci7e9p.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Wed, 09 Jul 2014 08:17:22 -0700")
Junio C Hamano <gitster@pobox.com> writes:
>> * Still, making sure that the (file, line) is doable later without too
>> much change is good. So, if indeed, moving all code to another file is
>> required, then it may make sense to do it now to avoid code move
>> later.
>
> Good thinking. As long as the code is prepared, it is a good idea
> to start small and bite off only we can chew at once, do things
> incrementally.
After thinking about it a bit more, I think <file, line> support
needs to be done not as a mere client of the generic, uncached
git_config() API that we have had for forever, like the current
iteration, but needs to know a bit more about the state the caller
of the callback (namely, git_parse_source()), and we obviously do
not want to expose that machinery to anybody other than the
implementation of the config subsystem (to which the new facility
this GSoC project adds belongs to), so in that sense you have to
have your code in the same config.c file anyway.
It is somewhat dissapointing that this initial implementation was
done as a client of the traditional git_config(), by the way. I
would have expected that it would be the other way around, in that
the traditional callers of git_config() would behefit automatically
by having the cache layer below it.
But we have to start from somewhere. Perhaps the round after this
one can rename the exisiting implementation of git_config() to
something else internal to the caching layer and give the existing
callers a compatible interface that is backed by this new caching
layer in a transparent way.
next prev parent reply other threads:[~2014-07-09 17:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-09 10:57 [PATCH v6 0/3] git config cache & special querying api utilizing the cache Tanay Abhra
2014-07-09 10:57 ` [PATCH v7 1/2] add `config_set` API for caching config-like files Tanay Abhra
2014-07-09 12:12 ` Matthieu Moy
2014-07-09 12:39 ` Tanay Abhra
2014-07-09 14:19 ` Matthieu Moy
2014-07-09 15:17 ` Junio C Hamano
2014-07-09 17:44 ` Junio C Hamano [this message]
2014-07-10 9:41 ` Matthieu Moy
2014-07-10 16:46 ` Junio C Hamano
2014-07-09 16:07 ` Tanay Abhra
2014-07-10 11:23 ` Matthieu Moy
2014-07-09 10:57 ` [PATCH v7 2/2] test-config: Add tests for the config_set API Tanay Abhra
2014-07-09 12:13 ` Matthieu Moy
2014-07-09 12:42 ` Tanay Abhra
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=xmqq38ea77gt.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=artagnon@gmail.com \
--cc=git@vger.kernel.org \
--cc=tanayabh@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.