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
> <sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> > Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > 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
Attachment:
signature.asc
Description: PGP signature