Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the model for parsing responses to a specific commit sha #302

Merged
merged 4 commits into from
Mar 29, 2023

Conversation

atilag
Copy link
Contributor

@atilag atilag commented Jan 25, 2023

The response from Github API to a request like:
https://api.github.com/repos/atilag/IBM-Quantum-Systems-Exercise/commits/026880557f1116052c504b5066d1ae90c7c5bcd0
Is something like this:

{
  "sha": "026880557f1116052c504b5066d1ae90c7c5bcd0",
  "node_id": "C_kwDOHOAX6doAKDAyNjg4MDU1N2YxMTE2MDUyYzUwNGI1MDY2ZDFhZTkwYzdjNWJjZDA",
  "commit": {
    "author": {
      "name": "Juan Gomez",
      "email": "[email protected]",
      "date": "2022-06-20T20:43:02Z"
    },
    "committer": {
     ...
    },
    ...
  "author": {
    "login": "atilag",
    "id": 1304071,
    "node_id": "MDQ6VXNlcjEzMDQwNzE=",
    "avatar_url": "https://avatars.githubusercontent.com/u/1304071?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/atilag",
    "html_url": "https://github.com/atilag",
    "followers_url": "https://api.github.com/users/atilag/followers",
    "following_url": "https://api.github.com/users/atilag/following{/other_user}",
    "gists_url": "https://api.github.com/users/atilag/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/atilag/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/atilag/subscriptions",
    "organizations_url": "https://api.github.com/users/atilag/orgs",
    "repos_url": "https://api.github.com/users/atilag/repos",
    "events_url": "https://api.github.com/users/atilag/events{/privacy}",
    "received_events_url": "https://api.github.com/users/atilag/received_events",
    "type": "User",
    "site_admin": false
  },
  ...
}

There are two kinds of "author" keys, but the octocrab model was confusing the one in the root level with the one under the "commit" key.
I have fixed the models, so the parser doesn't mix them up.
I have also changed the names to better match the responses' keys.

Nice work team!! It is a pleasure to contribute :)

The response from Github to something like:
https://api.github.com/repos/[owner]/[repo]/commits/[sha], has two type of Authors. Prior to these changes the "author" key in the root level has different fields than the "author" key from the praent "commit" key. This commit fixes the problem by properly naming the fields so the parser don't mix the two "author" keys.
Ok, this time I run the tests before commmiting :')
@maflcko
Copy link
Contributor

maflcko commented Mar 22, 2023

Needs conflicts solved if still relevant?

@atilag
Copy link
Contributor Author

atilag commented Mar 29, 2023

Yep, let me try fix them.

@atilag
Copy link
Contributor Author

atilag commented Mar 29, 2023

I still need these changes in order to consume the parts of the Github API related to commit information.
I'm for now maintaining my own fork, but I guess eventually someone else would need these changes too.

@XAMPPRocky
Copy link
Owner

Thank you for your PR, and congrats on your first contribution! 🎉 Sorry for the late review, I've been busy with a lot of other projects at the moment.

@XAMPPRocky XAMPPRocky merged commit a546dcf into XAMPPRocky:master Mar 29, 2023
@atilag
Copy link
Contributor Author

atilag commented Mar 29, 2023

No sorries! Thanks to you for sharing your excellent work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants