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

Intermediate commit issues enhancements #69

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

imlm
Copy link
Contributor

@imlm imlm commented Feb 17, 2014

Please throughouly review the changes, some of them overlap with features already implemented(i.e., I had already finished the issue fetching implementation in my branch) and I added a lot more metadata to the Commit class that might not be required, nonetheless, you can still use my changes as a reference for future implementations, for example, I fixed the issues/commits fetching to actually retrieve ALL the issues/commits.

imlm added 11 commits January 7, 2014 22:35
Reimplementing issues search using new Github API V3
Adding new issues search API to allow clients to choose the maximum
number of issues pages that should be returned
Adding new test cases for SearchGitHub.getAllProjectIssues(..) and
SearchGitHub.getProjectIssues(..)
Adding total additions and deletions count to Commits
Adding new CommitFiles to Commits that enables client to retrieve the
file names, per file addition/deletion/change counts, status and patch
string
to allow one to to get the Response object for a given request instead
of simply the response body
all issues for a user
Intermediate commit for the enhanced commits search API
getWithProtection methods to erroneously sleep for the predefined time
if the body of a response contained the "API rate limit exceeded for"
string even if the response was not a 403 - Forbidden due to an actual
rate limit overage.
@gustavopinto
Copy link
Member

Hi @imlm, thanks for your commits! But, before merge, can you provide more tests? In particular, for the new methods in the SeachGithub class.

@imlm
Copy link
Contributor Author

imlm commented Feb 18, 2014

For the get_ProjectCommit_ methods, it is important that you guys also evaluate whether the additional commit metadata that I added is important/relevant enough to stay there as fetching this metadata considerably increases the number of requests per get_ProjectCommit_ call. Without this metadata each get_ProjectCommit_ call will execute exactly number-of-pages API requests, with the additional metadata it will take number-of-pages requests + one request per commit, this might be a lot of requests per call!.

@imlm
Copy link
Contributor Author

imlm commented Feb 18, 2014

Definitely this requires some test code, I just didn't plan on doing it
myself, but I can do it as soon as I have some spare time! :)

2014-02-17 21:35 GMT-03:00 Gustavo [email protected]:

Hi @imlm https://github.com/imlm, thanks for your commits! But, before
merge, can you provide more tests? In particular, for the new methods in
the SeachGithub class.


Reply to this email directly or view it on GitHubhttps://github.com//pull/69#issuecomment-35336319
.

Irineu M.L. Moura

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.

2 participants