On 05/22/2015 12:10 AM, Eric Sunshine wrote:
On Thu, May 21, 2015 at 1:30 PM, karthik nayak <karthik.188@xxxxxxxxx> wrote:
On 05/21/2015 12:37 AM, Eric Sunshine wrote:
On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak <karthik.188@xxxxxxxxx>
wrote:
Makefile | 1 +
ref-filter.c | 73
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ref-filter.h | 47 ++++++++++++++++++++++++++++++++++++++
3 files changed, 121 insertions(+)
create mode 100644 ref-filter.c
create mode 100644 ref-filter.h
A shortcoming of this approach is that it's not blame-friendly.
Although those of us following this patch series know that much of the
code in this patch was copied from for-each-ref.c, git-blame will not
recognize this unless invoked in the very expensive "git blame -C -C
-C" fashion (if I understand correctly). The most blame-friendly way
to perform this re-organization is to have the code relocation (line
removals and line additions) occur in one patch.
There are multiple ways you could arrange to do so. One would be to
first have a patch which introduces just a skeleton of the intended
API, with do-nothing function implementations. A subsequent patch
would then relocate the code from for-each-ref.c to ref-filter.c, and
update for-each-ref.c to call into the new (now fleshed-out) API.
Did you read Junio's suggestion on how I should re-order this WIP patch
series ?
That's somewhat on the lines of what you're suggesting. I'll probably be
going ahead with that, not really sure about how blame works entirely so
what do you think about that?
Yes, Junio's response did a much better job of saying what I intended.
Also, his response said something I meant to mention but forgot:
namely that, to ease the review task, code movement should be pure
movement, and not involve other changes.
Anyhow, follow Junio's advice. He knows what he's talking about. ;-)
Alright, Thanks for clearing that out.
Regards,
Karthik
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html