On Mon, Aug 29, 2016 at 03:02:56PM +0800, Paul Tan wrote: > Hi Brian, > > On Mon, Aug 29, 2016 at 7:27 AM, brian m. carlson > <[email protected]> wrote: > > Signed-off-by: brian m. carlson <[email protected]> > > --- > > builtin/am.c | 138 > > +++++++++++++++++++++++++++++------------------------------ > > 1 file changed, 69 insertions(+), 69 deletions(-) > > I looked through this patch, and the conversion looks faithful and > straightforward to me. Just two minor comments: > > > diff --git a/builtin/am.c b/builtin/am.c > > index 739b34dc..632d4288 100644 > > --- a/builtin/am.c > > +++ b/builtin/am.c > > @@ -1053,10 +1053,10 @@ static void am_setup(struct am_state *state, enum > > patch_format patch_format, > > else > > write_state_text(state, "applying", ""); > > > > - if (!get_sha1("HEAD", curr_head)) { > > - write_state_text(state, "abort-safety", > > sha1_to_hex(curr_head)); > > + if (!get_oid("HEAD", &curr_head)) { > > + write_state_text(state, "abort-safety", > > oid_to_hex(&curr_head)); > > if (!state->rebasing) > > - update_ref("am", "ORIG_HEAD", curr_head, NULL, 0, > > + update_ref("am", "ORIG_HEAD", curr_head.hash, NULL, > > 0, > > UPDATE_REFS_DIE_ON_ERR); > > I noticed that you used update_ref_oid() in other places of this > patch. Perhaps this should use update_ref_oid() as well for > consistency?
I'll do that in a reroll.
> > @@ -1665,9 +1665,8 @@ static int fall_back_threeway(const struct am_state
> > *state, const char *index_pa
> > */
> > static void do_commit(const struct am_state *state)
> > {
> > - unsigned char tree[GIT_SHA1_RAWSZ], parent[GIT_SHA1_RAWSZ],
> > - commit[GIT_SHA1_RAWSZ];
> > - unsigned char *ptr;
> > + struct object_id tree, parent, commit;
> > + struct object_id *ptr;
>
> Ah, I just noticed that this is a very poorly named variable. Whoops.
> Since we are here, should we rename this to something like "old_oid"?
> Also, this should probably be a "const struct object_id *" as well, I
> think.
I'll fix that. Thanks for the review.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
