All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH 0/11] renaming argv_array
Date: Tue, 11 Aug 2020 14:28:32 -0400	[thread overview]
Message-ID: <20200811182832.GA33865@syl.lan> (raw)
In-Reply-To: <82991f30-fe37-d6d2-ffd5-8b0878f46c83@web.de>

On Tue, Aug 11, 2020 at 06:08:22PM +0200, René Scharfe wrote:
> Am 28.07.20 um 22:21 schrieb Jeff King:
> > The argv_array data type has turned out to be useful in our code base,
> > but the name isn't very good. From patch 2 of this series:
> >
> >   The name "argv-array" isn't very good, because it describes what the
> >   data type can be used for (program argument arrays), not what it
> >   actually is (a dynamically-growing string array that maintains a
> >   NULL-terminator invariant). This leads to people being hesitant to use
> >   it for other cases where it would actually be a good fit. The existing
> >   name is also clunky to use. It's overly long, and the name often leads
> >   to saying things like "argv.argv" (i.e., the field names overlap with
> >   variable names, since they're describing the use, not the type). Let's
> >   give it a more neutral name.
> >
> > This has bugged me for a while, so I decided to finally fix it. It
> > wasn't _too_ painful, though I'm sure there will be a little fallout
> > with topics in flight.
>
> Just as this landed in master now, https://lobste.rs/ decided to link to
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2493.pdf, which is a
> paper about reserved identifiers in C.  It contains a nice overview.
>
> Anyway, 7.31 of C11 says: "All external names described below are
> reserved no matter what headers are included by the program."  And
> 7.31.13 goes on: "Function names that begin with str, mem, or wcs and a
> lowercase letter may be added to the declarations in the <string.h>
> header."  So the names of the strvec functions are reserved.
>
> Also how about using Coccinelle and patience to reduce the impact of
> such a change next time?  I.e. adding the new thing, providing a
> semantic patch for converting old code, waiting a reasonable amount of
> time after the last conversion was necessary and then removing the
> old thing.

I don't think that this is a bad idea at all, but I'm also totally fine
with the approach that Peff took here. (Bear in mind that I'm saying
this as someone who didn't have any topics that used argv_array, and so
didn't have to do any conversion effort ;-)).

I worry a little bit about providing too long a grace period where we
have to worry about not introducing new uses of argv_array, and still
having to deal with some (perhaps less) fall-out later down the line.

For this change, I think it was perfectly fine to just rip the bandaid
off.

> René

Thanks,
Taylor

  reply	other threads:[~2020-08-11 18:28 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 20:21 Jeff King
2020-07-28 20:21 ` [PATCH 01/11] argv-array: use size_t for count and alloc Jeff King
2020-07-28 20:23 ` [PATCH 02/11] argv-array: rename to strvec Jeff King
2020-07-28 20:23 ` [PATCH 03/11] strvec: rename files from argv-array " Jeff King
2020-07-28 20:24 ` [PATCH 04/11] quote: rename sq_dequote_to_argv_array to mention strvec Jeff King
2020-07-28 20:24 ` [PATCH 05/11] strvec: convert builtin/ callers away from argv_array name Jeff King
2020-07-28 20:24 ` [PATCH 06/11] strvec: convert more " Jeff King
2020-07-28 20:25 ` [PATCH 07/11] strvec: convert remaining " Jeff King
2020-07-28 20:26 ` [PATCH 08/11] strvec: fix indentation in renamed calls Jeff King
2020-07-28 22:43   ` Jacob Keller
2020-07-28 23:31     ` Junio C Hamano
2020-07-28 20:26 ` [PATCH 09/11] strvec: update documention to avoid argv_array Jeff King
2020-07-28 20:27 ` [PATCH 10/11] strvec: drop argv_array compatibility layer Jeff King
2020-07-28 22:23   ` Junio C Hamano
2020-07-29  0:04     ` Jeff King
2020-07-29  0:37       ` Jeff King
2020-07-29  0:40         ` Jeff King
2020-07-29  0:47           ` Junio C Hamano
2020-07-29 16:54             ` Derrick Stolee
2020-07-29  0:44         ` Junio C Hamano
2020-07-29 16:22           ` Jeff King
2020-07-28 20:28 ` [PATCH 11/11] strvec: rename struct fields Jeff King
2020-07-28 21:16   ` Junio C Hamano
2020-07-28 21:18     ` Junio C Hamano
2020-07-29  6:55       ` Christian Couder
2020-07-29 16:34         ` Jeff King
2020-07-29 18:03           ` Junio C Hamano
2020-07-28 21:20     ` Jeff King
2020-07-28 22:45 ` [PATCH 0/11] renaming argv_array Jacob Keller
2020-07-29  0:06   ` Jeff King
2020-07-29  6:15 ` Christian Couder
2020-07-29  6:19   ` Christian Couder
2020-07-29 13:32   ` Eric Sunshine
2020-07-29 16:33   ` Jeff King
2020-08-11 16:08 ` René Scharfe
2020-08-11 18:28   ` Taylor Blau [this message]
2020-08-11 19:00   ` Junio C Hamano
2020-08-11 20:39     ` Jacob Keller
2020-08-11 21:03       ` Junio C Hamano
2020-08-12 12:42     ` Johannes Schindelin
2020-08-12 15:06   ` Jeff King
2020-08-12 15:10     ` Jeff King
2020-08-12 16:23       ` René Scharfe
2020-08-12 17:08         ` Jeff King
2020-08-12 18:18           ` René Scharfe
2020-08-12 19:57             ` 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=20200811182832.GA33865@syl.lan \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --subject='Re: [PATCH 0/11] renaming argv_array' \
    /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

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.