git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: santiago@nyu.edu, git@vger.kernel.org, sunshine@sunshineco.com,
	walters@verbum.org, Lukas Puehringer <luk.puehringer@gmail.com>
Subject: Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v
Date: Wed, 18 Jan 2017 10:49:24 -0800	[thread overview]
Message-ID: <xmqqziio9paj.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170118182843.iyico5jpii6a3z7i@sigill.intra.peff.net> (Jeff King's message of "Wed, 18 Jan 2017 13:28:44 -0500")

Jeff King <peff@peff.net> writes:

>> diff --git a/builtin/tag.c b/builtin/tag.c
>> index f81273a85a..fbb85ba3dc 100644
>> --- a/builtin/tag.c
>> +++ b/builtin/tag.c
>> @@ -66,10 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
>>  }
>>  
>>  typedef int (*each_tag_name_fn)(const char *name, const char *ref,
>> -				const unsigned char *sha1, void *cb_data);
>> +				const unsigned char *sha1, const void *cb_data);
>
> This would bite us later if one of the iterators really does need to
> pass something mutable. But as this iteration interface is confined to
> builtin/tag.c, I think it's a nice simple fix.
>
> A more general fix would be to pass a non-const pointer to const pointer
> (preferably inside a struct for readability). But I don't see any need
> for that complexity here.

My first trial was to loosen the constness of existing variable,
which was OK, but made me feel dirty by turning what does not need
to be mutable into mutable.  The iterator being local made me try
the other way and it turned out that currently there is no need for
mutable callback data ;-)

I agree that this may have to be updated, and if this were more
global thing, we'd better off doing so from the get-go, but for a
calling convention that is limited within a single file, I am more
comfortable saying we'll cross the bridge when we need to.

Thanks.

  reply	other threads:[~2017-01-18 18:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-17 23:37 [PATCH v6 0/6] Add --format to tag verification santiago
2017-01-17 23:37 ` [PATCH v6 1/6] gpg-interface,tag: add GPG_VERIFY_OMIT_STATUS flag santiago
2017-01-17 23:37 ` [PATCH v6 2/6] ref-filter: add function to print single ref_array_item santiago
2017-01-17 23:37 ` [PATCH v6 3/6] builtin/verify-tag: add --format to verify-tag santiago
2017-01-17 23:37 ` [PATCH v6 4/6] builtin/tag: add --format argument for tag -v santiago
2017-01-18  0:02   ` Junio C Hamano
2017-01-18  0:05   ` Junio C Hamano
2017-01-18  0:19   ` Junio C Hamano
2017-01-18 18:25     ` Junio C Hamano
2017-01-18 18:28       ` Santiago Torres
2017-01-18 18:44         ` Junio C Hamano
2017-01-18 18:50           ` Santiago Torres
2017-01-18 20:16           ` Eric Wong
2017-01-19  0:37             ` Eric Wong
2017-01-18 18:52         ` Junio C Hamano
2017-01-18 18:28       ` Jeff King
2017-01-18 18:49         ` Junio C Hamano [this message]
2017-01-17 23:37 ` [PATCH v6 5/6] t/t7030-verify-tag: Add --format specifier tests santiago
2017-01-17 23:37 ` [PATCH v6 6/6] t/t7004-tag: " santiago
2017-01-18  0:07 ` [PATCH v6 0/6] Add --format to tag verification 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

  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 \
    --in-reply-to=xmqqziio9paj.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=luk.puehringer@gmail.com \
    --cc=peff@peff.net \
    --cc=santiago@nyu.edu \
    --cc=sunshine@sunshineco.com \
    --cc=walters@verbum.org \
    --subject='Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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).