All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Keller\, Jacob E" <jacob.e.keller@intel.com>
Cc: "git\@vger.kernel.org" <git@vger.kernel.org>,
	"peff\@peff.net" <peff@peff.net>
Subject: Re: [PATCH 2/2 v4] tag: support configuring --sort via .gitconfig
Date: Fri, 11 Jul 2014 09:28:37 -0700	[thread overview]
Message-ID: <xmqqtx6n507e.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1405095622.22963.6.camel@jekeller-desk1.amr.corp.intel.com> (Jacob E. Keller's message of "Fri, 11 Jul 2014 16:20:23 +0000")

"Keller, Jacob E" <jacob.e.keller@intel.com> writes:

> On Fri, 2014-07-11 at 08:04 -0700, Junio C Hamano wrote:
> ...
>> > +static int tag_sort = 0;
>> 
>> Please do not initialize variables in bss segment to 0 by hand.
>> 
>> If this variable is meant to take one of these *CMP_SORT values
>> defined as macro later in this file, it is better to define this
>> variable somewhere after and close to the definitions of the macros.
>> Perhaps immediately after the "struct tag_filter" is declared?
>
> I put it just above the struct tag_filter now, as this puts it right
> below the #defines regarding it's value.

Either would be fine, but just to clarify.

Because these macro definitions are for the .sort field of that
structure, and the new tag_sort variable is the second user of that
macro, my suggestion to put it _after_ was to be in line with "add
new things at the end, when there is no compelling reason not to"
below.

>> When there is no reason to have things in a particular order, it is
>> customary to add new things at the end, not in the front, unless the
>> new thing is so much more important than everything else---but then
>> we are no longer talking about the case where there is no reason to
>> have things in a particular order ;-).

Thanks.

  reply	other threads:[~2014-07-11 16:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-10 23:52 [PATCH 1/2] tag: use skip_prefix instead of magic numbers Jacob Keller
2014-07-10 23:52 ` [PATCH 2/2 v4] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-11 15:04   ` Junio C Hamano
2014-07-11 16:20     ` Keller, Jacob E
2014-07-11 16:28       ` Junio C Hamano [this message]
2014-07-11 16:40     ` Keller, Jacob E
2014-07-11 18:29       ` Junio C Hamano
2014-07-11 20:37         ` Keller, Jacob E
2014-07-11 17:20     ` Keller, Jacob E
2014-07-16  0:55   ` 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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtx6n507e.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jacob.e.keller@intel.com \
    --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.