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

Improve Commits API #398

Open
cloud303-cholden opened this issue Jun 17, 2023 · 7 comments
Open

Improve Commits API #398

cloud303-cholden opened this issue Jun 17, 2023 · 7 comments

Comments

@cloud303-cholden
Copy link
Contributor

cloud303-cholden commented Jun 17, 2023

After some research, a possible solution to a discussion I started (link) is to list commits and send a get request for each one in order to get its additions and deletions. It looks like the only way to make this request is with octocrab::Octocrab::get. Would you be open to a PR that adds the "Get a commit" API (link)? It looks like the natural place to add it would be octocrab::commits::CommitHandler::get. Maybe it would also be useful to have a method for octocrab::commits::CommitHandler::list -> octocrab::commits::ListCommitsBuilder as well?

@XAMPPRocky
Copy link
Owner

Thank you for your issue! Yes adding that API would be appreciated. As for where it should go, I would look up the octokit JS API and see where they put it

@cloud303-cholden
Copy link
Contributor Author

Understood! I'll fork the repo and get started on it.

@cloud303-cholden
Copy link
Contributor Author

cloud303-cholden commented Jun 17, 2023

It looks like getCommit is under the repos API (link), similar to listCommits. I'm curious whether you prefer using repos or commits, because it looks like createCommitComment is also under repos (link), whereas your crate has it under commits at octocrab::commits::CommitHandler::create_comment. The GitHub docs suggest that the /commits endpoints are distinct from /repos, but they all ultimately fall under the/repos endpoint. If we put this API under repos, I'm guessing this would require a new GetRepoCommit model, since the response spec is different than the RepoCommit one.

@XAMPPRocky
Copy link
Owner

Ah yeah, I think it should go under commits, ultimately it’s very similar to repos because it does require you to specify a repo.

cloud303-cholden added a commit to cloud303-cholden/octocrab that referenced this issue Jul 8, 2023
Add `get` method to `CommitHandler`. This appears to more reliably
return `Some` for optional response fields such as `files` and `stats`.
See https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit
cloud303-cholden added a commit to cloud303-cholden/octocrab that referenced this issue Jul 10, 2023
XAMPPRocky pushed a commit that referenced this issue Jul 18, 2023
* Add get commit API (#398)

Add `get` method to `CommitHandler`. This appears to more reliably
return `Some` for optional response fields such as `files` and `stats`.
See https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit

* Format commit method and example (#398)
@JeanMertz
Copy link
Contributor

So, I spent a few hours yesterday figuring out why the additions and deletions fields weren't populated. I used the PullRequestHandler::list method.

Reading this issue, am I correct that some (all?) of the optional fields in PullRequest are only populated when you do a call to fetch the information for that individual Pull Request?

That's good to know. Is this documented somewhere, and I just missed that detail?

@JeanMertz
Copy link
Contributor

I just verified that this is indeed the case using:

gh api \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/OWNER/REPO/pulls/PULL_NUMBER

Without PULL_NUMBER, many details are missing from the listed pull requests.

@XAMPPRocky
Copy link
Owner

That's good to know. Is this documented somewhere, and I just missed that detail?

I don't know if it's documented anywhere in the GitHub API, but we should probably add a note to the list method stating it.

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

No branches or pull requests

3 participants