Re: [PATCH v2 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

2017-10-09 Thread Junio C Hamano
"brian m. carlson"  writes:

> All of the callers already pass the hash member of struct object_id, so
> update them to pass a pointer to the struct directly,
>
> This transformation was done with an update to declaration and
> definition and the following semantic patch:
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3.hash, E4)
> + resolve_refdup(E1, E2, , E4)
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3->hash, E4)
> + resolve_refdup(E1, E2, E3, E4)
>
> Signed-off-by: brian m. carlson 
> ---

As I mentioned in my response to the cover letter, quite a many of
these now pass NULL (i.e. "I do not care what commit the HEAD is at;
I only am interested in which branch I am on"), making major part of
this patch irrelevant.  The conflict resolution is trivial, and I do
not think rebasing is worth it, though.



Re: [PATCH v2 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

2017-10-09 Thread Jonathan Nieder
brian m. carlson wrote:

> All of the callers already pass the hash member of struct object_id, so
> update them to pass a pointer to the struct directly,
>
> This transformation was done with an update to declaration and
> definition and the following semantic patch:
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3.hash, E4)
> + resolve_refdup(E1, E2, , E4)
>
> @@
> expression E1, E2, E3, E4;
> @@
> - resolve_refdup(E1, E2, E3->hash, E4)
> + resolve_refdup(E1, E2, E3, E4)
>
> Signed-off-by: brian m. carlson 

Lovely.  I tried putting that in contrib/coccinelle/resolve_refdup.cocci,
running

  git grep -l -e resolve_refdup -- '*.c' '*.h' |
  xargs spatch --in-place --sp-file contrib/coccinelle/resolve_refdup.cocci

and diffing the result against this commit.  With --word-diff, there
are a small number of changes:

- the above semantic patch handles resolve_refdup but not
  refs_resolve_refdup.  This commit does both.

- as mentioned in the commit message, the above semantic patch only
  updates callers.  This commit updates the implementations to match.

Without --word-diff, I also see some line-wrapping changes, which all
seem reasonable.  (Coccinelle's line-wrapping heuristics seem to be
pretty specific to Linux kernel style.)

In other words, this does what it says on the cover in a
straightforward and reviewable way.  Thanks for that.

Reviewed-by: Jonathan Nieder