Skip to content

Add a variation of the getCommits method that supports Paging and a path #167

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 2 commits into from
Apr 4, 2018

Conversation

swilson11
Copy link
Contributor

Simple variation to make it easy to find all commits for a given file.

Simple variation to make it easy to find all commits for a given file.
@@ -180,6 +180,29 @@ public CommitsApi(GitLabApi gitLabApi) {
return (new Pager<Commit>(this, Commit.class, itemsPerPage, formData.asMap(), "projects", projectId, "repository", "commits"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could also modify this method to call the new one that I added, with null for the path, if you would prefer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this would be a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@gmessner
Copy link
Collaborator

gmessner commented Apr 4, 2018

@swilson11
Thank you for the contribution. I've added an inline comment. Additionally, could you add the following methods? It will make the additions consistent with the rest of the API:

public List<Commit> getCommits(int projectId, String ref, 
        Date since, Date until, String path) throws GitLabApiException

public List<Commit> getCommits(int projectId, String ref,
        Date since, Date until, String path, int page, int perPage) throws GitLabApiException

* @return a Pager containing the commits for the specified project ID
* @throws GitLabApiException GitLabApiException if any exception occurs during execution
*/
public Pager<Commit> getCommits(int projectId, String ref, Date since, Date until, int itemsPerPage, String path) throws GitLabApiException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move the path param before the itemsPerPage param? The itemsPerPage param should always be last.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -180,6 +180,29 @@ public CommitsApi(GitLabApi gitLabApi) {
return (new Pager<Commit>(this, Commit.class, itemsPerPage, formData.asMap(), "projects", projectId, "repository", "commits"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this would be a good idea.

Add another missing variant
Move path parameter before items per page
Have a previous variant call the new variant with a null path
@swilson11
Copy link
Contributor Author

For some reason, I can't reply to your comment above about adding the 2 new APIs. I added the second one, but the first one was already there. Thanks for maintaining this project, and for your quick responses on my request!

Sue

@gmessner
Copy link
Collaborator

gmessner commented Apr 4, 2018

Changes look good, and thanks again for your contribution.

@gmessner gmessner merged commit 94b37c5 into gitlab4j:master Apr 4, 2018
@gmessner
Copy link
Collaborator

gmessner commented Apr 5, 2018

@swilson11
This has been released in 4.8.9

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

Successfully merging this pull request may close these issues.

2 participants