git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: Parth Gala via GitGitGadget <gitgitgadget@gmail.com>,
	Git List <git@vger.kernel.org>, Parth Gala <parthpgala@gmail.com>
Subject: Re: [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter
Date: Wed, 12 Feb 2020 16:13:42 -0500
Message-ID: <CAPig+cQhh8pNppSsGGk62-VoHsOUv0aNTxHtDWY+_pousR+KMg@mail.gmail.com> (raw)
In-Reply-To: <20200212202210.GC4364@syl.local>

On Wed, Feb 12, 2020 at 3:22 PM Taylor Blau <me@ttaylorr.com> wrote:
> On Wed, Feb 12, 2020 at 07:19:07PM +0000, Parth Gala via GitGitGadget wrote:
> > diff --git a/builtin/fsck.c b/builtin/fsck.c
> > @@ -375,6 +375,7 @@ static void check_object(struct object *obj)
> >  static void check_connectivity(void)
> >  {
> >       int i, max;
> > +     struct repository *r = the_repository;
>
> Is there a reason that you assign use/assign 'r' here? [...]
> but I'm not sure that it's necessary here. Could you instead pass
> 'the_repository' directly to the functions that now require it?

One benefit of doing it this way is that future patches will be much
less noisy which convert these callers to also work with any
'repository' object. For instance, after the current patch, we have:

    static void check_connectivity(void)
    {
        struct repository *r = the_repository;
        max = get_max_object_index(r);
        ...

A future patch which converts check_connectivity() to work with any
repository will then require very little change -- just adding an 'r'
argument to the function declaration, and dropping the line which
declares 'r':

    static void check_connectivity(struct repository *r)
    {
        max = get_max_object_index(r);
        ...

So, I'm fine with this patch series' approach of declaring a variable
'r' like this.

  reply index

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12 19:19 [PATCH 0/5] object.c: localize global the_repository variable into r Parth Gala via GitGitGadget
2020-02-12 19:19 ` [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter Parth Gala via GitGitGadget
2020-02-12 20:22   ` Taylor Blau
2020-02-12 21:13     ` Eric Sunshine [this message]
2020-02-13  5:23     ` parth gala
2020-02-12 19:19 ` [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter Parth Gala via GitGitGadget
2020-02-12 20:25   ` Taylor Blau
2020-02-12 21:11     ` Junio C Hamano
2020-02-13 18:00       ` Taylor Blau
2020-02-13 18:10         ` Junio C Hamano
2020-02-13 18:52           ` Jeff King
2020-02-15  0:00             ` Taylor Blau
2020-02-12 19:19 ` [PATCH 3/5] object.c: parse_object_or_die() " Parth Gala via GitGitGadget
2020-02-12 19:19 ` [PATCH 4/5] object.c: clear_object_flags() " Parth Gala via GitGitGadget
2020-02-12 19:19 ` [PATCH 5/5] object.c: clear_commit_marks_all() " Parth Gala via GitGitGadget
2020-02-12 20:18 ` [PATCH 0/5] object.c: localize global the_repository variable into r Taylor Blau
2020-02-13  5:14   ` parth gala

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=CAPig+cQhh8pNppSsGGk62-VoHsOUv0aNTxHtDWY+_pousR+KMg@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=parthpgala@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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git