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
next prev parent 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.