git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Git List" <git@vger.kernel.org>, "Jeff King" <peff@peff.net>,
	"René Scharfe" <l.s.r@web.de>,
	"Andreas Schwab" <schwab@linux-m68k.org>,
	"Johannes Sixt" <j6t@kdbg.org>
Subject: Re: [PATCH] strbuf: use designated initializers in STRBUF_INIT
Date: Fri, 14 Jul 2017 10:13:50 -0700	[thread overview]
Message-ID: <CAGZ79kaFH5BXEsTEz55NbfhrJoWfutTVQi-bv3NpomCoK9OzWw@mail.gmail.com> (raw)
In-Reply-To: <xmqqlgnrq9qi.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 14, 2017 at 9:11 AM, Junio C Hamano <gitster@pobox.com> wrote:

>  - I prefer to keep decl-after-statement out of our codebase.  I
>    view it as a big plus in code-readability to be able to see a
>    complete list of variables that will be used in a block upfront
>    before starting to read the code that uses them.

sounds good to me.

>  - Corollary to the above, I do not mind to have a variable
>    declaration in the initialization clause of a for() statement
>    (e.g. "for (int i = 0; i < 4; i++) { ... }"), as the scoping rule
>    is very sensible.  Some of our "for()" statements use the value
>    of the variable after iteration, for which this new construct
>    cannot be used, though.

I might send a test balloon for this.

>    . // comments

I would think these are useful for two reasons:
(a) these seem to be used widely outside of these old-style project
  (git, gcc, kernel), such that most new contributors need to be told
  to avoid these which adds to the entry burden.
(b) recursive nesting of comments is possible. We may not need this
  in the final patch, but I sure use that in development. To comment out
  a whole function containing multiple comments I have to select all
  lines and prefix them with // currently. If all comments were //, I could
  put /* .. */ around the function, which seems easier. I just realize
  I can use #if 0 .. #endif though.

  (a) may be a concern, (b) not so much from the projects point of view.

>    . restricted pointers

That sounds like the ultimate micro optimization, but may be hard
to reason about after a years of churn, so it may become hard to
maintain.

>    . static and type qualifiers in parameter array declarators

That sounds equally arcane.

> -- >8 --
> Subject: [PATCH] clean.c: use designated initializer
>
> This is another test balloon to see if we get complaints from people
> whose compilers do not support designated initializer for arrays.

This sounds as if it is applied on top of Jeffs test balloon patch, such
that we do not need to re-iterate the criteria why to do it here, e.g.
this code is always compiled and did not change a lot over the last
couple month, so potentially easy to revert.

Thanks,
Stefan

  reply	other threads:[~2017-07-14 17:13 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-10  7:03 [PATCH] strbuf: use designated initializers in STRBUF_INIT Jeff King
2017-07-10 14:57 ` Ben Peart
2017-07-10 16:04   ` Jeff King
2017-07-10 17:57     ` Ben Peart
2017-07-11  5:01   ` Mike Hommey
2017-07-11 15:31   ` Junio C Hamano
2017-07-12 19:12     ` Ævar Arnfjörð Bjarmason
2017-07-12 21:08       ` Junio C Hamano
2017-07-13 22:24       ` Johannes Sixt
2017-07-10 16:44 ` Junio C Hamano
2017-07-10 17:33   ` Stefan Beller
2017-07-10 21:46     ` Junio C Hamano
2017-07-10 17:10 ` Andreas Schwab
2017-07-10 19:57 ` Johannes Sixt
2017-07-10 20:38   ` Junio C Hamano
2017-07-10 21:11     ` Johannes Sixt
2017-07-10 21:38       ` Junio C Hamano
2017-07-14 16:11         ` Junio C Hamano
2017-07-14 17:13           ` Stefan Beller [this message]
2017-07-14 17:36           ` Jeff King
2017-07-14 18:48             ` Junio C Hamano
2017-07-14 19:16               ` Junio C Hamano
2017-07-19 18:19                 ` [PATCH] objects: scope count variable to loop Stefan Beller
2017-07-19 18:23                   ` Brandon Williams
2017-07-24 17:08                     ` Jeff King
2017-07-24 17:12                       ` Stefan Beller
2017-07-24 18:05                         ` Jeff King
2017-07-14 19:28           ` [PATCH] strbuf: use designated initializers in STRBUF_INIT Ævar Arnfjörð Bjarmason
2017-07-14 22:26             ` Junio C Hamano
2017-07-14 22:43           ` Mike Hommey
2017-07-15 11:08             ` Jeff King
2017-07-11  4:38       ` Jeff King
2017-07-11  0:05   ` brian m. carlson
2017-07-11  0:07     ` Stefan Beller
2017-07-11  0:10       ` brian m. carlson
2017-07-11  5:24     ` Johannes Sixt
2017-07-12  1:26       ` brian m. carlson
2017-07-12 18:25         ` Johannes Sixt
2017-07-10 22:41 ` Brandon Williams

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=CAGZ79kaFH5BXEsTEz55NbfhrJoWfutTVQi-bv3NpomCoK9OzWw@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=schwab@linux-m68k.org \
    /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.
Code repositories for project(s) associated with this public 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).