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: Heiko Voigt <hvoigt@hvoigt.net>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Jens Lehmann <jens.lehmann@web.de>,
	Jonathan Nieder <jrnieder@gmail.com>, Jeff King <peff@peff.net>,
	"W. Trevor King" <wking@tremily.us>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Karsten Blees <karsten.blees@gmail.com>
Subject: Re: [PATCH v5 0/4] submodule config lookup API
Date: Mon, 10 Aug 2015 12:23:08 -0700	[thread overview]
Message-ID: <CAGZ79kakGg6Ejworq5xVr2QuzLHxh=E6tzU_PoW+0M6AWuKJfg@mail.gmail.com> (raw)
In-Reply-To: <xmqq8ubk7idb.fsf@gitster.dls.corp.google.com>

On Mon, Jun 15, 2015 at 2:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks.  Will replace and wait for comments from others.

I have reviewed the patches carefully and they look good to me.

As Git is a large project and I was active in other parts until now,
I noticed that there are subtle differences in style as when compared
to the refs code. One example would be the way comments are written.
In d378e35d256348f (Patch 1, implement submodule config API for
lookup of .gitmodules values) the comments for the data structures in
submodule-config.c seem to have a non exposed "headline" and if more
is needed proper sentences with capitalized starts and punctuation at the
end. In the refs code there are only sentences IIRC. Most of the commits
touching submodule.{c,h} do not prefix their commit message with
"submodule:"

The style is no show stopper of course, just an observation from someone
moving into a different area of code.

Thanks,
Stefan

  reply	other threads:[~2015-08-10 19:23 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-15 21:06 [PATCH v5 0/4] submodule config lookup API Heiko Voigt
2015-06-15 21:06 ` [PATCH v5 1/4] implement submodule config API for lookup of .gitmodules values Heiko Voigt
2015-06-16 10:54   ` Heiko Voigt
2015-07-08 20:52   ` Phil Hord
2015-07-09 12:09     ` Heiko Voigt
2015-07-09 15:49       ` Jeff King
2015-07-09 19:41         ` Jens Lehmann
2015-07-09 20:00           ` Junio C Hamano
2015-07-13 11:17             ` Heiko Voigt
2015-07-13 15:49               ` Junio C Hamano
2015-06-15 21:06 ` [PATCH v5 2/4] extract functions for submodule config set and lookup Heiko Voigt
2015-06-15 21:06 ` [PATCH v5 3/4] use new config API for worktree configurations of submodules Heiko Voigt
2015-06-15 21:06 ` [PATCH v5 4/4] do not die on error of parsing fetchrecursesubmodules option Heiko Voigt
2015-06-15 21:48 ` [PATCH v5 0/4] submodule config lookup API Junio C Hamano
2015-08-10 19:23   ` Stefan Beller [this message]
2015-08-12 17:53     ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2015-08-12 19:13 Stefan Beller
2015-08-12 19:27 ` Junio C Hamano

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='CAGZ79kakGg6Ejworq5xVr2QuzLHxh=E6tzU_PoW+0M6AWuKJfg@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hvoigt@hvoigt.net \
    --cc=jens.lehmann@web.de \
    --cc=jrnieder@gmail.com \
    --cc=karsten.blees@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=wking@tremily.us \
    /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).