git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Ramsay Jones <ramsay@ramsayjones.plus.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH] attr: mark a file-local symbol as static
Date: Wed, 18 Jan 2017 15:05:41 -0800	[thread overview]
Message-ID: <20170118230541.GE10641@google.com> (raw)
In-Reply-To: <89290015-7c5f-1a5d-e683-59077ae55bf5@ramsayjones.plus.com>

On 01/18, Ramsay Jones wrote:
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> 
> Hi Brandon,
> 
> If you need to re-roll your 'bw/attr' branch, could you please
> squash this into the relevant patch (commit 8908457159,
> "attr: use hashmap for attribute dictionary", 12-01-2017).
> 
> Also, I note that, although they are declared as part of the
> public attr api, attr_check_clear() and attr_check_reset() are
> also not called outside of attr.c. Are these functions part of
> the public api?
> 
> Also, a minor point, but attr_check_reset() is called (line 1050)
> before it's definition (line 1114). This is not a problem, given
> the declaration in attr.h, but I prefer definitions to come before
> use, where possible.
> 
> Thanks!
> 
> ATB,
> Ramsay Jones

Yes of course, I believe Stefan also pointed that out earlier today so I
have it fixed locally.

For attr_check_clear() and attr_check_reset() the intent is that they
are the accepted way to either clear or reset the attr_check structure.
Currently most users of the attribute system don't have a need to clear
or reset the structure but there could be future callers who need that
functionality.  If you feel like they shouldn't be part of the api right
now then I'm open to changing that for this series.

Thanks!

-- 
Brandon Williams

  reply	other threads:[~2017-01-18 23:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-18 22:27 [PATCH] attr: mark a file-local symbol as static Ramsay Jones
2017-01-18 23:05 ` Brandon Williams [this message]
2017-01-19  1:22   ` Ramsay Jones
  -- strict thread matches above, loose matches on Subject: below --
2016-11-13 16:42 Ramsay Jones
2016-11-14 20:00 ` 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=20170118230541.GE10641@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsayjones.plus.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).