Re: On blame/pickaxe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <junkio@xxxxxxx> writes:

> 4. Passing more blame.
>
> Instead of taking responsibility for the remainder, there are
> other ways to find other people to pass blame on.  That's what
> the "NEEDSWORK" comment in pass_blame() is about.

I've spent a few hours tonight to further work (eh, "have fun")
on this.  The version at the tip of "pu" implements detection of
a case like this:

> A typical example is a change that moves one static C function
> from lower part of the file to upper part of the same file,
> because you added a new caller in the middle.  The path in your
> parent and the path in you would look like this:
>
>         parent                          you
>
>         A                               static foo() {
>         B                               }
>         C                               A
>         D                               B
>         E                               C
>         F                               D
>         G                               ... call foo();
>         static foo() {                  E
>         }                               F
>         I                               G
>         J                               I
>
> With the common part finding code with diff, you will be able to
> pass blames for lines A B C D E F G I J to your parent.  You are
> truly guilty for introducing "... call foo();".  The problem
> here is that in addition, you will be blamed for the lines that
> implement "static foo() { ... }" at the beginning of your file.

You can use the pickaxe on its source itself, like this:

	git pickaxe -n master..pu builtin-pickaxe.c

If you compare this with output from:

	git log --pretty=short -p master..pu builtin-pickaxe.c

you will notice the line-movement detection in action.

During the course of development, I had to move quite a few
static functions around so that they are defined before their
first call site.  This is partly because I am very bad at
initial planning (who is?) and this still being in experimental
stage I did not bother declaring static functions upfront as
forward declarations.

For example, commit db3f0f2 introduces find_last_in_target()
function, but it was moved up by commit b5c0e4f (near the tip of
"pu").  pickaxe blames the implementation of it to db3f0f20, and
also notices that the bulk of its logic was actually copied from
the implementation of pass_blame_to_parent() function in commit
b14dc9ef (the initial commit that introduced builtin-pickaxe.c).

What _ought_ to come next is to detect line movement across
files, but I'll go to bed for now.

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]