All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4alt/4] attr: drop DEBUG_ATTR code
Date: Thu, 6 Oct 2022 14:33:08 -0400	[thread overview]
Message-ID: <fb4cfbd3-e738-f1e8-18f7-d4e90050b8b5@github.com> (raw)
In-Reply-To: <xmqqwn9clwba.fsf@gitster.g>

On 10/6/22 1:02 PM, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
>> On Thu, Oct 06, 2022 at 09:13:41AM -0400, Jeff King wrote:
>>
>>> The other obvious option is to just delete this debug code, and remove
>>> the unused parameter. I'm not sure if the trace would ever be useful or
>>> not, and I am mostly retaining it out of the logic of "well, somebody
>>> bothered to write it". I think the const issue has been there since
>>> e810e06357 (attr: tighten const correctness with git_attr and
>>> match_attr, 2017-01-27).
>>
>> And here's what that would look like.
> 
> I highly suspect that I was the one who bothered, and while I admit
> it was useful while developing the attribute subsystem, I haven't
> needed it for the past 10 or so years.
> 
> So unless there are some folks who want to throw everything into the
> trace2 floodstream, I would prefer this alternative over the other
> one.

Are you implying that you want to use the second version, that
deletes the information entirely? I'm leaning towards deleting
it.

If not, and we should keep using traces, I do notice that the
original version of the patch uses trace_printf_key() instead
of a trace2 method. I think this is fine, too, since it's
likely only to be used by Git developers, who could look for
which type of trace to use.

Thanks,
-Stolee

  reply	other threads:[~2022-10-06 18:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-06 13:09 [PATCH 0/4] a few small unused-parameter fixes Jeff King
2022-10-06 13:10 ` [PATCH 1/4] test-submodule: inline resolve_relative_url() function Jeff King
2022-10-06 13:10 ` [PATCH 2/4] multi-pack-index: avoid writing to global in option callback Jeff King
2022-10-06 13:11 ` [PATCH 3/4] commit: " Jeff King
2022-10-06 13:13 ` [PATCH 4/4] attr: convert DEBUG_ATTR to use trace API Jeff King
2022-10-06 13:23   ` [PATCH 4alt/4] attr: drop DEBUG_ATTR code Jeff King
2022-10-06 17:02     ` Junio C Hamano
2022-10-06 18:33       ` Derrick Stolee [this message]
2022-10-06 18:52         ` Junio C Hamano
2022-10-11  0:26         ` Jeff King
2022-10-11  0:23       ` Jeff King

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=fb4cfbd3-e738-f1e8-18f7-d4e90050b8b5@github.com \
    --to=derrickstolee@github.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.