fix(436): Fix Missing/wrong pull requests for github commits

[dark0dave]

Description

fixes #436

Motivation and Context

How Has This Been Tested?

Screenshots / Logs (if applicable)

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (no code change)
• Refactor (refactoring production code)
• Other

Checklist:

• My code follows the code style of this project.
• I have updated the documentation accordingly.
• I have formatted the code with rustfmt.
• I checked the lints with clippy.
• I have added tests to cover my changes.
• All new and existing tests passed.

[dark0dave]

these will be removed don't worry

[orhun]

Looks good, do we need to remove the macro after this? Or what is in your mind?

Also, let me know if it is ready for testing 🙂

[dark0dave]

Looks good, do we need to remove the macro after this? Or what is in your mind?

Also, let me know if it is ready for testing 🙂'

For now keep the macro, whats on my mind currently is do we want to mark all the commits in a pr as contributors?

Example:

Old head                   *
                              \
I open pr 1 commit             *
                               |
You add a commit               *
                             /    
Alice merges commit        *       (merge commit)

Should I be a contributor as well as you? Currently I am just taking the commit before the merge commit ( ie in the above example you would be the contributor).

We don't need the to poll the api to get the pr commit history. The parent commits of the pr allow us to work backwards via the tree and discover the other relevant commits for that pr.

[orhun]

Should I be a contributor as well as you? Currently I am just taking the commit before the merge commit ( ie in the above example you would be the contributor).

Yes, I think that's reasonable.

We don't need the to poll the api to get the pr commit history. The parent commits of the pr allow us to work backwards via the tree and discover the other relevant commits for that pr.

Hmm, interesting. So we don't need to send 2 requests? Can you show me an example?

[dark0dave]

Should I be a contributor as well as you? Currently I am just taking the commit before the merge commit ( ie in the above example you would be the contributor).

Yes, I think that's reasonable.

We don't need the to poll the api to get the pr commit history. The parent commits of the pr allow us to work backwards via the tree and discover the other relevant commits for that pr.

Hmm, interesting. So we don't need to send 2 requests? Can you show me an example?

Let me clarify: We do need to list PRs and commits. We do not need to send a request for each prs commit history.

The github commit object exposes a parents field:

https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-commits

We can use this parents field to traverse the commit history, git history is after all a tree.

    "parents": [
      {
        "url": "https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e",
        "sha": "6dcb09b5b57875f334f61aebed695e2e4193db5e"
      }
    ]

Ie the above example with Alice, you and I. The Alice's merge commits parents are:

• old head
• your commit

Your commit's parent are:

• my commit

My commit parent are:

• old head

[orhun]

Let me clarify: We do need to list PRs and commits. We do not need to send a request for each prs commit history.

wait, we don't do that already. We only fetch all the commits and PRs & try to link between them using the available fields (merge_commit_sha etc.)

Ie the above example with Alice, you and I. The Alice's merge commits parents are:

I see, I think it would be better to talk over the implementation :) Let me know when this is ready for review.

[orhun]

ping :)

[dark0dave]

@orhun so sorry I have been traveling for work. I'll ping you in a bit.