git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] clean up extern decl of functions
Date: Fri, 09 Oct 2020 12:26:49 -0700	[thread overview]
Message-ID: <xmqqy2kfryiu.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: xmqqtuv4uncn.fsf@gitster.c.googlers.com

Junio C Hamano <gitster@pobox.com> writes:

>> Why are we re-introducing an explicit "extern"? Since function decls are
>> extern by default, what do we gain by doing this?
>>
>> You mentioned in the past[0]
>>
>> 	I think there is a push to drop the "extern " from decls of
>> 	functions in *.h header files.
>>
>> so are we reversing that push now?
>
> That is certainly on the table.  Re-read what you quoted and realize
> that I was not expressing my opinion on the "push"; it was just
> stating that other reviewers seem to be in favor.
>
> See my other response why I think the "push"  was a bad idea.

I'd elaborate a bit more.  A proposed update to CodingGuidelines
will be sent separately as a follow-up to this message.

"Are we reversing that push now?"  That is not a question I can
unilaterally answer yes/no---I do not run dictatorship where I
cannot survive without telling all the contributors how to cross
their t's and dot their i's.  There are things in our coding
guidelines that tells me to do something differently from how I
would, but I can adjust and survive if the primary benefit of having
guidelines, i.e. making things uniform one way or the other, is net
win.  When a guideline turns out to be a bad idea, however, I can
propose to change it.  So can you or anybody else ;-)

In that message, I just told Emily that there is a push to omit
extern, in the sense that it was the opinion of the prevailing
louder voices.  Back then, I didn't have an opinion strong enough to
favor either way myself, and I was willing to go with the majority
if many contributors wanted to drop "extern" in the hope that it
will result in quality code.

But with the Makefile patch you posted with Dscho's review this
morning, it has become apparent to me that it wasn't a great idea
after all.  It caused you to spend your time to write the RFC patch
and come up with the regexp, and caused the project to spend
reviewer bandwidth on the patch.

Of course, how you spend your time is entirely up to you.  But if
you are going to contribute your time on this project, the project
appreciates if the time is spent on things that make the codebase
better.  And to me, unlike to me "in the past[0]", it is reasonably
clear that the push of omitting "extern" ended up wasting resources
without doing much good for the project.

Seeing that the pattern that were trying to be careful didn't catch
the decls fixed by the patch you were responding to in this thread
did not help improve my impression on the idea of omitting "extern".

I think these two decls I touched in this patch were left behind
when somebody, possibly you in b199d714 (*.[ch]: remove extern from
function declarations using sed, 2019-04-29), tried to "clean up"
the last time, because the pattern used in the conversion did not
catch it.

  reply	other threads:[~2020-10-09 19:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-08 15:27 [PATCH] clean up extern decl of functions Junio C Hamano
2020-10-09  1:55 ` Denton Liu
2020-10-09  2:47   ` Junio C Hamano
2020-10-09 19:26     ` Junio C Hamano [this message]
2020-10-09 19:27       ` [RFC] CodingGuidelines: mark external declarations with "extern" Junio C Hamano
2020-10-09 19:57         ` Jeff King
2020-10-09 20:33           ` Junio C Hamano
2020-10-09 23:00             ` Denton Liu
2020-10-09 23:05               ` Junio C Hamano
2020-10-10  0:37             ` Đoàn Trần Công Danh
2020-10-15  1:36             ` Jeff King
2020-10-15 17:15               ` Junio C Hamano
2020-10-15 19:28                 ` Jeff King
2020-10-15 19:30                   ` [PATCH v2 1/2] usage: define a type for a reporting function Jeff King
2020-10-15 19:30                   ` [PATCH v2 2/2] config.mak.dev: build with -fno-common 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=xmqqy2kfryiu.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=liu.denton@gmail.com \
    /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 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 NNTP newsgroup(s).