From 4539f80aa44e500422c5071d8e3d74de321b6225 Mon Sep 17 00:00:00 2001 From: Nat Williams Date: Tue, 17 Apr 2012 17:35:31 -0500 Subject: add pull request API there are a few little issues remaining. Mostly regarding handling meaningful non-20x response codes --- pygithub3/services/pull_requests/__init__.py | 129 +++++++++++++++++++++++++++ pygithub3/services/pull_requests/comments.py | 73 +++++++++++++++ 2 files changed, 202 insertions(+) create mode 100644 pygithub3/services/pull_requests/__init__.py create mode 100644 pygithub3/services/pull_requests/comments.py (limited to 'pygithub3/services') diff --git a/pygithub3/services/pull_requests/__init__.py b/pygithub3/services/pull_requests/__init__.py new file mode 100644 index 0000000..2197f60 --- /dev/null +++ b/pygithub3/services/pull_requests/__init__.py @@ -0,0 +1,129 @@ +from pygithub3.exceptions import BadRequest, NotFound +from pygithub3.services.base import Service, MimeTypeMixin +from .comments import Comments + + +class PullRequests(Service, MimeTypeMixin): + """Consume `Pull Request API `_""" + + def __init__(self, **config): + self.comments = Comments(**config) + super(PullRequests, self).__init__(**config) + + def list(self, user=None, repo=None): + """List all of the pull requests for a repo + + :param str user: Username + :param str repo: Repository + + """ + return self._get_result( + self.make_request('pull_requests.list', user=user, repo=repo) + ) + + def get(self, number, user=None, repo=None): + """Get a single pull request + + :param str number: The number of the pull request to get + :param str user: Username + :param str repo: Repository + + """ + return self._get( + self.make_request('pull_requests.get', number=number, user=user, + repo=repo) + ) + + def create(self, body, user=None, repo=None): + """Create a pull request + + :param dict body: Data for the new pull request + :param str user: Username + :param str repo: Repository + + """ + return self._post( + self.make_request('pull_requests.create', body=body, user=user, + repo=repo) + ) + + def update(self, number, body, user=None, repo=None): + """Update a pull request + + :param str number: The number of the the pull request to update + :param dict body: The data to update the pull request with + :param str user: Username + :param str repo: Repository + + """ + return self._patch( + self.make_request('pull_requests.update', number=number, + body=body, user=user, repo=repo) + ) + + def list_commits(self, number, user=None, repo=None): + """List the commits for a pull request + + :param str number: The number of the pull request to list commits for + :param str user: Username + :param str repo: Repository + + """ + return self._get_result( + self.make_request('pull_requests.list_commits', number=number, + user=user, repo=repo) + ) + + def list_files(self, number, user=None, repo=None): + """List the files for a pull request + + :param str number: The number of the pull request to list files for + :param str user: Username + :param str repo: Repository + + """ + return self._get_result( + self.make_request('pull_requests.list_files', number=number, + user=user, repo=repo) + ) + + def merge_status(self, number, user=None, repo=None): + """Gets whether a pull request has been merged or not. + + :param str number: The pull request to check + :param str user: Username + :param str repo: Repository + + """ + # for this to work with a proper Resource, we would need to pass the + # response's status code to the Resource constructor, and that's kind + # of scary + try: + resp = self._client.get( + self.make_request('pull_requests.merge_status', number=number, + user=user, repo=repo) + ) + except NotFound: + return False + code = resp.status_code + if code == 204: + return True + # TODO: more flexible way to return arbitrary objects based on + # response. Probably something on Request + raise BadRequest('got code %s: %s' % (code, resp.content)) + # again, I'm sorry. + + def merge(self, number, message='', user=None, repo=None): + """Merge a pull request. + + :param str number: The pull request to merge + :param str user: Username + :param str repo: Repository + + """ + # so, the API docs don't actually say what the status code will be in + # the case of a merge failure. I hope it's not a 404. + return self._put( + self.make_request('pull_requests.merge', number=number, + message=message, user=user, repo=repo) + ) diff --git a/pygithub3/services/pull_requests/comments.py b/pygithub3/services/pull_requests/comments.py new file mode 100644 index 0000000..3aa6d0e --- /dev/null +++ b/pygithub3/services/pull_requests/comments.py @@ -0,0 +1,73 @@ +from pygithub3.services.base import Service, MimeTypeMixin + + +class Comments(Service, MimeTypeMixin): + """Consume `Review Comments API + `_ + + """ + + def list(self, number, user=None, repo=None): + """List all the comments for a pull request + + :param str number: The number of the pull request + :param str user: Username + :param str repo: Repository + + """ + return self._get_result( + self.make_request('pull_requests.comments.list', number=number, + user=user, repo=repo) + ) + + def get(self, number, user=None, repo=None): + """Get a single comment + + :param str number: The comment to get + :param str user: Username + :param str repo: Repository + + """ + return self._get( + self.make_request('pull_requests.comments.get', number=number, + user=user, repo=repo) + ) + + def create(self, number, body, user=None, repo=None): + """Create a comment + + :param str number: the pull request to comment on + :param str user: Username + :param str repo: Repository + + """ + return self._post( + self.make_request('pull_requests.comments.create', number=number, + body=body, user=user, repo=repo) + ) + + def edit(self, number, body, user=None, repo=None): + """Edit a comment + + :param str number: The id of the comment to edit + :param str user: Username + :param str repo: Repository + + """ + return self._patch( + self.make_request('pull_requests.comments.edit', number=number, + body=body, user=user, repo=repo) + ) + + def delete(self, number, user=None, repo=None): + """Delete a comment + + :param str number: The comment to delete + :param str user: Username + :param str repo: Repository + + """ + return self._delete( + self.make_request('pull_requests.comments.delete', number=number, + user=user, repo=repo) + ) -- cgit v1.2.3-59-g8ed1b From d1f86894f91b21c7c92a9154caf3815f930aa8a6 Mon Sep 17 00:00:00 2001 From: Nat Williams Date: Wed, 18 Apr 2012 14:14:55 -0500 Subject: pass body to pull_requests.merge properly --- pygithub3/services/pull_requests/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'pygithub3/services') diff --git a/pygithub3/services/pull_requests/__init__.py b/pygithub3/services/pull_requests/__init__.py index 2197f60..f7da50a 100644 --- a/pygithub3/services/pull_requests/__init__.py +++ b/pygithub3/services/pull_requests/__init__.py @@ -120,10 +120,12 @@ class PullRequests(Service, MimeTypeMixin): :param str user: Username :param str repo: Repository + This currently raises an HTTP 405 error if the request is not + mergable. + """ - # so, the API docs don't actually say what the status code will be in - # the case of a merge failure. I hope it's not a 404. + body = {'commit_message': message} return self._put( self.make_request('pull_requests.merge', number=number, - message=message, user=user, repo=repo) + body=body, user=user, repo=repo) ) -- cgit v1.2.3-59-g8ed1b From 9008296715194c150e7eeb16469005d771f7d370 Mon Sep 17 00:00:00 2001 From: Nat Williams Date: Thu, 19 Apr 2012 09:57:34 -0500 Subject: use _bool for pull request merge status --- pygithub3/services/pull_requests/__init__.py | 21 ++++----------------- pygithub3/tests/services/test_pull_requests.py | 4 ++-- 2 files changed, 6 insertions(+), 19 deletions(-) (limited to 'pygithub3/services') diff --git a/pygithub3/services/pull_requests/__init__.py b/pygithub3/services/pull_requests/__init__.py index f7da50a..66d9e58 100644 --- a/pygithub3/services/pull_requests/__init__.py +++ b/pygithub3/services/pull_requests/__init__.py @@ -95,23 +95,10 @@ class PullRequests(Service, MimeTypeMixin): :param str repo: Repository """ - # for this to work with a proper Resource, we would need to pass the - # response's status code to the Resource constructor, and that's kind - # of scary - try: - resp = self._client.get( - self.make_request('pull_requests.merge_status', number=number, - user=user, repo=repo) - ) - except NotFound: - return False - code = resp.status_code - if code == 204: - return True - # TODO: more flexible way to return arbitrary objects based on - # response. Probably something on Request - raise BadRequest('got code %s: %s' % (code, resp.content)) - # again, I'm sorry. + return self._bool( + self.make_request('pull_requests.merge_status', number=number, + user=user, repo=repo) + ) def merge(self, number, message='', user=None, repo=None): """Merge a pull request. diff --git a/pygithub3/tests/services/test_pull_requests.py b/pygithub3/tests/services/test_pull_requests.py index 8071b09..7fc15b7 100644 --- a/pygithub3/tests/services/test_pull_requests.py +++ b/pygithub3/tests/services/test_pull_requests.py @@ -113,7 +113,7 @@ class TestPullRequestsService(TestCase): self.assertEqual(True, resp) self.assertEqual( reqm.call_args[0], - ('get', _('repos/user/repo/pulls/123/merge')) + ('head', _('repos/user/repo/pulls/123/merge')) ) def test_MERGE_STATUS_false(self, reqm): @@ -122,7 +122,7 @@ class TestPullRequestsService(TestCase): self.assertEqual(False, resp) self.assertEqual( reqm.call_args[0], - ('get', _('repos/user/repo/pulls/123/merge')) + ('head', _('repos/user/repo/pulls/123/merge')) ) def test_MERGE(self, reqm): -- cgit v1.2.3-59-g8ed1b