git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
	"git\@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH] refs.h: rename submodule arguments to submodule_path
Date: Thu, 20 Apr 2017 18:12:04 -0700	[thread overview]
Message-ID: <xmqqy3uu7ee3.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAGZ79kYTqh=Qa+Pt1+MojrcYFr05HQgbPRcc=DvjCkUWsjP5Uw@mail.gmail.com> (Stefan Beller's message of "Thu, 20 Apr 2017 11:21:04 -0700")

Stefan Beller <sbeller@google.com> writes:

> + Junio

Just like Michael, I do not have strong enough opinion for or
against this patch to comment on it.

I do agree with you that it would be a good longer-term direction to
use "submodule" for a "struct submodule" (i.e. submodule object),
and call a string that names a submodule either "submodule_name" or
"submodule_path" depending on how it names one, for maintainability
of the code.  

However I am not convinced that this patch is an improvement.  Even
though parameter names in decls only serve documentation purpose and
it is even OK to only have type without name there, if we are going
to _have_ names, it would make sense to match them to the parameter
names actually used in the implementation.  

Updating these names used in refs.c would make a very noisy patch,
of course.  But I am not sure if it is a good middle ground to avoid
that and to update only refs.h.


  reply	other threads:[~2017-04-21  1:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-12 20:00 [PATCH] refs.h: rename submodule arguments to submodule_path Stefan Beller
2017-04-20 18:21 ` Stefan Beller
2017-04-21  1:12   ` Junio C Hamano [this message]
2017-04-21  6:32     ` Michael Haggerty
2017-04-21  6:42       ` Michael Haggerty
2017-04-21  9:38         ` Duy Nguyen

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=xmqqy3uu7ee3.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=sbeller@google.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.
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).