Skip to content

attribution is not always correct for pull requests#21

Merged
lalitkapoor merged 4 commits intomasterfrom
21-attribution-is-not-always-correct-for-pull-requests
Feb 28, 2014
Merged

attribution is not always correct for pull requests#21
lalitkapoor merged 4 commits intomasterfrom
21-attribution-is-not-always-correct-for-pull-requests

Conversation

@lalitkapoor
Copy link
Copy Markdown
Owner

this is because we don't have enough information in just the merge commit. We need to explore all the commits in the branch and give credit to all the authors who have a commit in the branch.

Relying on the user name from the commit message is especially invalid for organization accounts.

Note: Should be fine for --data pulls

Issue was discovered in #20 thanks.

@lalitkapoor lalitkapoor self-assigned this Feb 26, 2014
@lalitkapoor
Copy link
Copy Markdown
Owner Author

first I'm going to need to find common ancestor for the parents of the merge commit to determine where to stop traversing the graph for authors. Then I need to follow parents back to ancestor node, aggregating unique authors along the way.

Note: merges can have multiple parents, so need to support more than just 2 parents.

@lalitkapoor
Copy link
Copy Markdown
Owner Author

Hmm, finding common ancestor is no problem, but determining all authors might be tricky if the graph gets complex, because some paths might not lead back to the common ancestor. Maybe I can go about it another way.

Example of such a graph:

*   8995d17 - (HEAD, master) Merge branch 'f' (2 seconds ago) <Lalit Kapoor>
|\
| *   4c849d8 - (f) Merge branch 'd' into f (46 seconds ago) <Lalit Kapoor>
| |\
| | * cd74de4 - (d) d3 (8 minutes ago) <Lalit Kapoor>
| | * ecf6caf - d2 (8 minutes ago) <Lalit Kapoor>
| | * b48beec - d1 (9 minutes ago) <Lalit Kapoor>
* | | dce8c66 - h1 (12 seconds ago) <Lalit Kapoor>
* | | dbd467a - g1 (22 seconds ago) <Lalit Kapoor>
|/ /
* | 49479ed - e1 (2 minutes ago) <Lalit Kapoor>
|/
*-.   ff4be83 - (origin/master, origin/HEAD, h, g, e) Merge branches 'a', 'b' and 'c' (2 hours ago) <Lalit Kapoor>

@lalitkapoor
Copy link
Copy Markdown
Owner Author

Notes from IRC:

 <idefine_> is there a way to get a list of the commits that came in during a merge?

 <Nevik> idefine_: yes, but you'll need to know (or find out) which way the merge occurred

 <idefine_> Nevik, lets assume I have that information, how would i figure that out?

 <Nevik> idefine_: `git log <merge commit> --not <your own side's parent of the merge>`

 <idefine_> Nevik, could you please explain what happened by using --not. I'm sorry I didn't grasp it form the docs

 <Nevik> idefine_: `git log <commit>` shows you all commits reachable from <commit>; `git log <commit1> <commit2> ...` shows you all commits reachable from *any* of the commits you give it; `git log <commit1> --not <commit2>` shows you all commits reachable from commit1 which are NOT reachable from commit 2

 <Nevik> idefine_: everything reachable from a merge commit, which is no reachable from your own side of the merge, is exactly "what the merge brought in"

 <idefine_> Nevik, Makes a lot more sense to me now. Thank you so much for the clarification.

 <Nevik> idefine_: sure :)

 <idefine_> Nevik, I have another question, in relation. How do you determine 'your own side's parent of the merge'? Is it always the first parent (if there are multiple) ?

 <thiago> idefine_: the one that was checked out is always the first parent

 <Nevik> idefine_: which of the parents is the first depends on which side the merge was made from, as i said. cf. what thiago just said

 <Nevik> so it depends on whether you merged in the othe rbranch, or if the other branch merged in yours

 <idefine_> thiago, Nevik: just to make sure I've got it - is this correct: so if I merge X into master then the first parent is that of master's. if I merge master into X then the first parent is that of X.

 <Nevik> idefine_: almost. if you merge X into master (you have master checked out and do `git merge X`), master becomes the first parent of the merge (i.e. the commit where master currently points, before the merge is completely, at which time the master branch is moved to point to the new commit)
 * psoo_ has quit (Client Quit)

 <Nevik> idefine_: analogously, the other way would make (the current) X the first parent of the merge

Note to self: So if I merge X into master then the first parent is that of the previous commit on master. If I merge master into X then the first parent is that of the previous commit on X.

lalitkapoor added a commit that referenced this pull request Feb 28, 2014
…correct-for-pull-requests

attribution is not always correct for pull requests
@lalitkapoor lalitkapoor merged commit 43244f8 into master Feb 28, 2014
@lalitkapoor lalitkapoor deleted the 21-attribution-is-not-always-correct-for-pull-requests branch February 28, 2014 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant