Skip to content

Conversation

@halfdanthemild
Copy link
Contributor

@halfdanthemild halfdanthemild commented Jul 2, 2025

In models::repo::Commit, author - which is of type CommitAuthor (https://github.com/XAMPPRocky/octocrab/blob/main/src/models/repos.rs#L160) - has a non-optional name field. But this was causing crashes for me with the error Serde error: missing field 'name' as Github does not return the author name here. name is also not present in the example response in the Github docs for this endpoint either (https://docs.github.com/en/rest/search/search?apiVersion=2022-11-28#search-commits):

"author": {
  "login": "octocat",
  "id": 583231,
  "node_id": "MDQ6VXNlcjU4MzIzMQ==",
  "avatar_url": "https://avatars.githubusercontent.com/u/583231?v=3",
  "gravatar_id": "",
  "url": "https://api.github.com/users/octocat",
  "html_url": "https://github.com/octocat",
  "followers_url": "https://api.github.com/users/octocat/followers",
  "following_url": "https://api.github.com/users/octocat/following{/other_user}",
  "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}",
  "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}",
  "subscriptions_url": "https://api.github.com/users/octocat/subscriptions",
  "organizations_url": "https://api.github.com/users/octocat/orgs",
  "repos_url": "https://api.github.com/users/octocat/repos",
  "events_url": "https://api.github.com/users/octocat/events{/privacy}",
  "received_events_url": "https://api.github.com/users/octocat/received_events",
  "type": "User",
  "site_admin": false
}

Using models::commits::Commit instead seems to work.

@halfdanthemild halfdanthemild marked this pull request as ready for review July 2, 2025 11:29
@maflcko
Copy link
Contributor

maflcko commented Aug 1, 2025

lgtm

I've tested this via:

    octocrab::instance()
        .search()
        .commits("clippy repo:XAMPPRocky/octocrab")
        .sort("author-date")
        .order("desc")
        .send()
        .await
        .unwrap();

Previously, it failed. Now, it passes.

I haven't checked if the full schema is implemented correctly, but if there are issues again in the future, a new specific SearchCommit struct could be added.

But this lgtm as-is.

@XAMPPRocky
Copy link
Owner

Thank you for your PR, and congrats on your first contribution! 🎉

@XAMPPRocky XAMPPRocky merged commit 1ca87c2 into XAMPPRocky:main Aug 1, 2025
20 checks passed
@halfdanthemild halfdanthemild deleted the serde-commit-fix branch August 4, 2025 09:03
@github-actions github-actions bot mentioned this pull request Aug 1, 2025
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