All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: Jeff King <peff@peff.net>
Cc: git <git@vger.kernel.org>
Subject: Re: [PATCH] CodingGuidelines: mention "static" and "extern"
Date: Thu, 8 Feb 2018 13:43:13 -0800	[thread overview]
Message-ID: <CAGZ79kYjHMYO19bJUQNWXyiwOp3HwiiAYuOHO9O7VnxUzzC1Tg@mail.gmail.com> (raw)
In-Reply-To: <20180208213806.GA6381@sigill.intra.peff.net>

On Thu, Feb 8, 2018 at 1:38 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 08, 2018 at 01:04:08PM -0800, Stefan Beller wrote:
>
>> You may sense a pattern here: I currently have the very firm understanding
>> we use the extern keyword in our codebase.
>>
>> And I can also attest that this was not always the case, as back in the
>> day I remember writing patches without the extern keyword only to be told:
>> (A) be similar to the function in the next lines
>> (B) the standard is to use extern
>> and I was convinced it was a bad decision to prefix declarations with
>> the extern keyword, but followed along as I don't want to have style
>> in the way of writing features.
>
> It definitely was the case that people used to suggest "extern". I think
> this was a Linus-ism from the early days, and I have been hating it for
> almost 12 years now. ;)
>
>>   $ cat Documentation/CodingGuidelines |grep extern
>>   $ # oh no it's empty!
>>
>> Care to add a section to our coding guidelines?
>
> Here's a patch.
>
> -- >8 --
> Subject: [PATCH] CodingGuidelines: mention "static" and "extern"
>
> It perhaps goes without saying that file-local stuff should
> be marked static, but it does not hurt to remind people.
>
> Less obvious is that we are settling on "do not include
> extern in function declarations". It is already the default
> unless the function was previously declared static (but if
> you are following a static declaration with an unmarked one,
> you should think about why you are declaring the thing
> twice). And so it just becomes an extra noise-word in our
> header files.
>
> We used to give the opposite advice, so there are quite a
> few "extern" markers in early Git code. But this at least
> makes a concrete suggestion that we can follow going
> forward.
>
> Signed-off-by: Jeff King <peff@peff.net>

Reviewed-by: Stefan Beller <sbeller@google.com>

... and now I can resend that patch, after fixing it to
follow our style. :)

> ---
>  Documentation/CodingGuidelines | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index c4cb5ff0d4..48aa4edfbd 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -386,6 +386,11 @@ For C programs:
>   - Use Git's gettext wrappers to make the user interface
>     translatable. See "Marking strings for translation" in po/README.
>
> + - Variables and functions local to a given source file should be marked
> +   with "static". Variables that are visible to other source files
> +   must be declared with "extern" in header files. However, function
> +   declarations should not use "extern", as that is already the default.
> +
>  For Perl programs:
>
>   - Most of the C guidelines above apply.
> --
> 2.16.1.365.g89f5777adf
>

  reply	other threads:[~2018-02-08 21:43 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08 20:15 [PATCH] color.h: document and modernize header Stefan Beller
2018-02-08 20:43 ` Jeff King
2018-02-08 21:04   ` Stefan Beller
2018-02-08 21:38     ` [PATCH] CodingGuidelines: mention "static" and "extern" Jeff King
2018-02-08 21:43       ` Stefan Beller [this message]
2018-02-08 23:14       ` Eric Sunshine
2018-02-09 19:07         ` Jonathan Tan
2018-02-09 19:33           ` Jeff King
2018-02-08 22:26   ` [PATCH] color.h: document and modernize header Eric Sunshine
2018-02-08 22:28     ` Jeff King
2018-02-12 20:19       ` Stefan Beller
2018-02-12 22:14         ` Eric Sunshine
2018-02-13  1:41           ` Stefan Beller
2018-02-13  3:55             ` Eric Sunshine
2018-02-14  7:23               ` Eric Sunshine
2018-02-14 17:58                 ` Stefan Beller

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=CAGZ79kYjHMYO19bJUQNWXyiwOp3HwiiAYuOHO9O7VnxUzzC1Tg@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --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.