Re: [PATCH 10/35] commit: add repository argument to lookup_commit
On 06/14, Stefan Beller wrote:
> On Thu, Jun 14, 2018 at 9:22 AM Duy Nguyen wrote:
> >
> > On Wed, May 30, 2018 at 2:51 AM Stefan Beller wrote:
> > > diff --git a/shallow.c b/shallow.c
> > > index 9bb07a56dca..60fe1fe1e58 100644
> > > --- a/shallow.c
> > > +++ b/shallow.c
> > > @@ -31,7 +31,7 @@ int register_shallow(struct repository *r, const struct
> > > object_id *oid)
> > > {
> > > struct commit_graft *graft =
> > > xmalloc(sizeof(struct commit_graft));
> > > - struct commit *commit = lookup_commit(oid);
> > > + struct commit *commit = lookup_commit(the_repository, oid);
> >
> > This looks wrong. register_shallow() has struct repository argument
> > 'r' and it should be used here instead.
>
> Right.
>
> > If this is a mechanical conversion, I will also be happy that the
> > switch from the_repo to r is done in a separate patch.
>
> This part of the code is not touched later in this series,
> so I'll fix it if a reroll is needed.
Yeah maybe at some point when lookup_commit can understand arbitrary
repositories we can change this from the_repository to r. This patch is
part of that mechanical change and has to be the_repository till
lookup_commit has been fully converted.
>
> > FYI I noticed this because I'm in a quest to kill the_index by passing
> > 'struct index_state *' throughout library code, and sometimes I pass
> > 'struct repository *' instead when I see that code uses more things
> > that just the index. And I have started to replace the_repository in
> > some places with a function argument.
> >
> > If some of my patches come first while you have not finished
> > repository conversion (very likely), you and I will have to pay
> > attention to this more often.
--
Brandon Williams
Re: [PATCH 10/35] commit: add repository argument to lookup_commit
On Thu, Jun 14, 2018 at 9:22 AM Duy Nguyen wrote:
>
> On Wed, May 30, 2018 at 2:51 AM Stefan Beller wrote:
> > diff --git a/shallow.c b/shallow.c
> > index 9bb07a56dca..60fe1fe1e58 100644
> > --- a/shallow.c
> > +++ b/shallow.c
> > @@ -31,7 +31,7 @@ int register_shallow(struct repository *r, const struct
> > object_id *oid)
> > {
> > struct commit_graft *graft =
> > xmalloc(sizeof(struct commit_graft));
> > - struct commit *commit = lookup_commit(oid);
> > + struct commit *commit = lookup_commit(the_repository, oid);
>
> This looks wrong. register_shallow() has struct repository argument
> 'r' and it should be used here instead.
Right.
> If this is a mechanical conversion, I will also be happy that the
> switch from the_repo to r is done in a separate patch.
This part of the code is not touched later in this series,
so I'll fix it if a reroll is needed.
> FYI I noticed this because I'm in a quest to kill the_index by passing
> 'struct index_state *' throughout library code, and sometimes I pass
> 'struct repository *' instead when I see that code uses more things
> that just the index. And I have started to replace the_repository in
> some places with a function argument.
>
> If some of my patches come first while you have not finished
> repository conversion (very likely), you and I will have to pay
> attention to this more often.
Re: [PATCH 10/35] commit: add repository argument to lookup_commit
On Wed, May 30, 2018 at 2:51 AM Stefan Beller wrote:
> diff --git a/shallow.c b/shallow.c
> index 9bb07a56dca..60fe1fe1e58 100644
> --- a/shallow.c
> +++ b/shallow.c
> @@ -31,7 +31,7 @@ int register_shallow(struct repository *r, const struct
> object_id *oid)
> {
> struct commit_graft *graft =
> xmalloc(sizeof(struct commit_graft));
> - struct commit *commit = lookup_commit(oid);
> + struct commit *commit = lookup_commit(the_repository, oid);
This looks wrong. register_shallow() has struct repository argument
'r' and it should be used here instead.
If this is a mechanical conversion, I will also be happy that the
switch from the_repo to r is done in a separate patch.
FYI I noticed this because I'm in a quest to kill the_index by passing
'struct index_state *' throughout library code, and sometimes I pass
'struct repository *' instead when I see that code uses more things
that just the index. And I have started to replace the_repository in
some places with a function argument.
If some of my patches come first while you have not finished
repository conversion (very likely), you and I will have to pay
attention to this more often.
--
Duy
