All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git <git@vger.kernel.org>
Subject: Re: [PATCH 01/11] string_list: print_string_list to use trace_printf
Date: Fri, 7 Sep 2018 10:21:57 -0700	[thread overview]
Message-ID: <CAGZ79kbObDXyaRLADQpvMMSitOLCDwb030Q6UBiX-7Y-XWnqog@mail.gmail.com> (raw)
In-Reply-To: <xmqqo9d9y6oy.fsf@gitster-ct.c.googlers.com>

On Fri, Sep 7, 2018 at 2:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > I guess the question is: is this a thing we would want to make available
> > to code to leave in all the time? Or is it just for sticking in
> > temporarily for a quick dump?
> >
> > If the former, then I think it needs the early-return at the least (and
> > probably _should_ have the key parameter).
> >
> > But I get the feeling that you really just want the latter, and are only
> > grudgingly being pushed into the former by Junio's suggestion.

That is not quite the case. I did take all but the one suggestion
as I think they are good suggestions and I appreciate it.

However having the trace key is not useful I would think.
(I did not need it for debugging, but presented an API
of what I would have found useful)

If the key is really that useful, I would just think we can add it on top.

> Well, if that is the case, I'd change "my suggestion" (although I
> didn't mean to suggest anything concrete).  If this was needed
> and/or was useful only during the defvelopment of the remainder of
> the series, and if this is known to be a half-hearted change that is
> not meant to be useful as a general solution, then let's not take
> the main part of the series a hostage to this step; rather, we
> should drop this change and leave it to another series that is
> willing to do it right.  It would save both author's and reviewers'
> time if we did so.

In that case, would we be interested in a separate patch
that kills off that function instead?

Stefan

  reply	other threads:[~2018-09-07 17:22 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 23:01 [PATCH 00/11] fetch: make sure submodule oids are fetched Stefan Beller
2018-09-04 23:01 ` [PATCH 01/11] string_list: print_string_list to use trace_printf Stefan Beller
2018-09-06 16:52   ` Junio C Hamano
2018-09-06 16:56     ` Jeff King
2018-09-06 22:16       ` Stefan Beller
2018-09-07  0:04         ` Jeff King
2018-09-07  9:53           ` Junio C Hamano
2018-09-07 17:21             ` Stefan Beller [this message]
2018-09-10 21:58               ` [PATCH 1/2] trace: add trace_print_string_list_key Stefan Beller
2018-09-10 21:58                 ` [PATCH 2/2] string-list: remove unused function print_string_list Stefan Beller
2018-09-10 22:32                 ` [PATCH 1/2] trace: add trace_print_string_list_key Junio C Hamano
2018-09-10 22:38                   ` Stefan Beller
2018-09-11  0:52                     ` Junio C Hamano
2018-09-11  3:08                       ` Stefan Beller
2018-09-11 21:03                         ` Jeff King
2018-09-11 18:48                       ` [PATCH] string-list: remove unused function print_string_list Stefan Beller
2018-09-11 19:27                         ` Junio C Hamano
2018-09-11 19:30                           ` Junio C Hamano
2018-09-11 19:47                             ` Stefan Beller
2018-09-11 20:53                               ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 02/11] string-list.h: add string_list_{pop, last} functions Stefan Beller
2018-09-06 17:10   ` Junio C Hamano
2018-09-06 22:29     ` Stefan Beller
2018-09-04 23:01 ` [PATCH 03/11] sha1-array: provide oid_array_filter Stefan Beller
2018-09-06 17:20   ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 04/11] submodule.c: convert submodule_move_head new argument to object id Stefan Beller
2018-09-06 17:31   ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 05/11] submodule.c: fix indentation Stefan Beller
2018-09-06 17:34   ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 06/11] submodule.c: sort changed_submodule_names before searching it Stefan Beller
2018-09-06 18:03   ` Junio C Hamano
2018-09-11 18:31     ` Stefan Beller
2018-09-04 23:01 ` [PATCH 07/11] submodule: move global changed_submodule_names into fetch submodule struct Stefan Beller
2018-09-06 18:04   ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 08/11] submodule.c: do not copy around submodule list Stefan Beller
2018-09-06 18:20   ` Junio C Hamano
2018-09-04 23:01 ` [PATCH 09/11] submodule: fetch in submodules git directory instead of in worktree Stefan Beller
2018-09-04 23:01 ` [PATCH 10/11] fetch: retry fetching submodules if sha1 were not fetched Stefan Beller
2018-09-04 23:01 ` [PATCH 11/11] builtin/fetch: check for submodule updates for non branch fetches Stefan Beller

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=CAGZ79kbObDXyaRLADQpvMMSitOLCDwb030Q6UBiX-7Y-XWnqog@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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.
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.