Skip to content

api: add GET /api/v1/{owner}/{repo}/commits endpoint #6574

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

Merged
merged 4 commits into from
Sep 23, 2021
Merged

api: add GET /api/v1/{owner}/{repo}/commits endpoint #6574

merged 4 commits into from
Sep 23, 2021

Conversation

jaylevin
Copy link
Contributor

@jaylevin jaylevin commented Jun 7, 2021

This pull request targets issue #6573.

It provides a new API endpoint: /api/v1/repos/{org}/{repo}/commits?pageSize=<int> with a default page size of 30 commits (the same as the UI).

This implementation currently only focuses on the main/master branch of the repository, and does not provide the ability to return commit history for other branches.

  • Note: Since the logic for converting a git.Commit to api.Commit had to be used in GetAllCommits and GetSingleCommit, I decided to pull the code out into a helper function, gitCommitToAPICommit(commit, context).

@jaylevin
Copy link
Contributor Author

jaylevin commented Jun 14, 2021

Hi @unknwon,

Is there any way I can get a review/approval for this PR?

Best,
Jordan

@unknwon unknwon self-requested a review September 23, 2021 14:43
Copy link
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Left few comments.

@unknwon unknwon added status: needs followup Need some extra work status: reviewed It has been reviewed by maintainers labels Sep 23, 2021
@jaylevin
Copy link
Contributor Author

Thanks for the PR! Left few comments.

Thank you for the review! The comments should be addressed 👍

@jaylevin jaylevin requested a review from unknwon September 23, 2021 15:04
Copy link
Member

@unknwon unknwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the followup!

@unknwon unknwon removed the status: needs followup Need some extra work label Sep 23, 2021
@unknwon unknwon changed the title GET /api/v1/{owner}/{repo}/commits endpoint (read all commits) api: add GET /api/v1/{owner}/{repo}/commits endpoint Sep 23, 2021
@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #6574 (4d2eaf7) into main (b3eb33b) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main   #6574   +/-   ##
=====================================
  Coverage   8.82%   8.82%           
=====================================
  Files         96      96           
  Lines      13194   13194           
=====================================
  Hits        1165    1165           
  Misses     11867   11867           
  Partials     162     162           

@unknwon
Copy link
Member

unknwon commented Sep 23, 2021

The lint is failing for things that are not related to this PR, I will merge and fix it in another commit.

@unknwon unknwon merged commit b9a3626 into gogs:main Sep 23, 2021
@jaylevin jaylevin deleted the read-commits branch December 9, 2021 17:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: reviewed It has been reviewed by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants