Skip to content

Commit 24a3ed5

Browse files
copituxcopitux
committed
Some fixes/typos and 'validate_body' related
1 parent 29d43d1 commit 24a3ed5

File tree

8 files changed

+84
-53
lines changed

8 files changed

+84
-53
lines changed

‎docs/pull_requests.rst‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ Pull Requests service
77

88
from pygithub3 import Github
99

10-
gh = Github()
10+
gh = Github(user='octocat', repo='sample')
1111

1212
pull_requests = gh.pull_requests.list().all()
13-
for pr in pull_requests:
14-
commits = gh.pull_requests.list_commits(pr.number).all()
13+
pull_request_commits = gh.pull_requests.list_commits(2512).all()
1514

1615
Pull Requests
1716
-------------
@@ -31,3 +30,6 @@ Pull Request Comments
3130

3231
.. autoclass:: pygithub3.services.pull_requests.Comments
3332
:members:
33+
34+
.. _github pullrequests doc: http://developer.github.com/v3/pulls
35+
.. _github pullrequests comments doc: http://developer.github.com/v3/pulls/comments

‎pygithub3/core/client.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def request(self, verb, request, **kwargs):
8181

8282
def get(self, request, **kwargs):
8383
response = self.request('get', request, **kwargs)
84-
# there are valid GET responses that != 200
84+
assert response.status_code == 200
8585
return response
8686

8787
def post(self, request, **kwargs):

‎pygithub3/requests/pull_requests/__init__.py‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
# -*- encoding: utf-8 -*-
2+
13
from pygithub3.requests.base import Request, ValidationError
2-
from pygithub3.resources.base import Raw
34
from pygithub3.resources.pull_requests import PullRequest, File
45
from pygithub3.resources.repos import Commit
56

@@ -22,11 +23,12 @@ class Create(Request):
2223
'required': ('base', 'head'),
2324
}
2425

25-
def validate_body(self, parsed):
26-
if (not ('title' in parsed and 'body' in parsed) and
27-
not 'issue' in parsed):
26+
def clean_body(self):
27+
if (not ('title' in self.body and 'body' in self.body) and
28+
not 'issue' in self.body):
2829
raise ValidationError('pull request creation requires either an '
2930
'issue number or a title and body')
31+
return self.body
3032

3133
class Update(Request):
3234
uri = 'repos/{user}/{repo}/pulls/{number}'
@@ -36,10 +38,12 @@ class Update(Request):
3638
'required': (),
3739
}
3840

39-
def validate_body(self, body):
40-
if 'state' in body and body['state'] not in ['open', 'closed']:
41+
def clean_body(self):
42+
if ('state' in self.body and
43+
self.body['state'] not in ['open', 'closed']):
4144
raise ValidationError('If a state is specified, it must be one '
4245
'of "open" or "closed"')
46+
return self.body
4347

4448

4549
class List_commits(Request):
@@ -52,14 +56,12 @@ class List_files(Request):
5256
resource = File
5357

5458

55-
class Merge_status(Request):
59+
class Is_merged(Request):
5660
uri = 'repos/{user}/{repo}/pulls/{number}/merge'
57-
resource = Raw
5861

5962

6063
class Merge(Request):
6164
uri = 'repos/{user}/{repo}/pulls/{number}/merge'
62-
resource = Raw
6365
body_schema = {
6466
'schema': ('commit_message',),
6567
'required': (),

‎pygithub3/requests/pull_requests/comments.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
from pygithub3.requests.base import Request
1+
# -*- encoding: utf-8 -*-
2+
3+
from pygithub3.requests.base import Request, ValidationError
24
from pygithub3.resources.pull_requests import Comment
35

46

@@ -20,13 +22,14 @@ class Create(Request):
2022
'required': ('body',),
2123
}
2224

23-
def validate_body(self, body):
24-
if (not ('commit_id' in body and
25-
'path' in body and
26-
'position' in body) and
27-
not 'in_reply_to' in body):
25+
def clean_body(self):
26+
if (not ('commit_id' in self.body and
27+
'path' in self.body and
28+
'position' in self.body) and
29+
not 'in_reply_to' in self.body):
2830
raise ValidationError('supply either in_reply_to or commit_id, '
2931
'path, and position')
32+
return self.body
3033

3134

3235
class Edit(Request):

‎pygithub3/resources/pull_requests.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# -*- encoding: utf-8 -*-
2+
13
from .base import Resource
24

35

‎pygithub3/services/pull_requests/__init__.py‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
from pygithub3.exceptions import BadRequest, NotFound
1+
# -*- encoding: utf-8 -*-
2+
23
from pygithub3.services.base import Service, MimeTypeMixin
34
from .comments import Comments
45

@@ -15,7 +16,10 @@ def list(self, user=None, repo=None):
1516
1617
:param str user: Username
1718
:param str repo: Repository
19+
:returns: A :doc:`result`
1820
21+
.. note::
22+
Remember :ref:`config precedence`
1923
"""
2024
return self._get_result(
2125
self.make_request('pull_requests.list', user=user, repo=repo)
@@ -28,37 +32,43 @@ def get(self, number, user=None, repo=None):
2832
:param str user: Username
2933
:param str repo: Repository
3034
35+
.. note::
36+
Remember :ref:`config precedence`
3137
"""
3238
return self._get(
3339
self.make_request('pull_requests.get', number=number, user=user,
3440
repo=repo)
3541
)
3642

37-
def create(self, body, user=None, repo=None):
43+
def create(self, data, user=None, repo=None):
3844
"""Create a pull request
3945
40-
:param dict body: Data for the new pull request
46+
:param dict data: Input. See `github pullrequests doc`_
4147
:param str user: Username
4248
:param str repo: Repository
4349
50+
.. note::
51+
Remember :ref:`config precedence`
4452
"""
4553
return self._post(
46-
self.make_request('pull_requests.create', body=body, user=user,
54+
self.make_request('pull_requests.create', body=data, user=user,
4755
repo=repo)
4856
)
4957

50-
def update(self, number, body, user=None, repo=None):
58+
def update(self, number, data, user=None, repo=None):
5159
"""Update a pull request
5260
5361
:param str number: The number of the the pull request to update
54-
:param dict body: The data to update the pull request with
62+
:param dict data: Input. See `github pullrequests doc`_
5563
:param str user: Username
5664
:param str repo: Repository
5765
66+
.. note::
67+
Remember :ref:`config precedence`
5868
"""
5969
return self._patch(
6070
self.make_request('pull_requests.update', number=number,
61-
body=body, user=user, repo=repo)
71+
body=data, user=user, repo=repo)
6272
)
6373

6474
def list_commits(self, number, user=None, repo=None):
@@ -67,7 +77,10 @@ def list_commits(self, number, user=None, repo=None):
6777
:param str number: The number of the pull request to list commits for
6878
:param str user: Username
6979
:param str repo: Repository
80+
:returns: A :doc:`result`
7081
82+
.. note::
83+
Remember :ref:`config precedence`
7184
"""
7285
return self._get_result(
7386
self.make_request('pull_requests.list_commits', number=number,
@@ -80,33 +93,42 @@ def list_files(self, number, user=None, repo=None):
8093
:param str number: The number of the pull request to list files for
8194
:param str user: Username
8295
:param str repo: Repository
96+
:returns: A :doc:`result`
8397
98+
.. note::
99+
Remember :ref:`config precedence`
84100
"""
85101
return self._get_result(
86102
self.make_request('pull_requests.list_files', number=number,
87103
user=user, repo=repo)
88104
)
89105

90-
def merge_status(self, number, user=None, repo=None):
106+
def is_merged(self, number, user=None, repo=None):
91107
"""Gets whether a pull request has been merged or not.
92108
93109
:param str number: The pull request to check
94110
:param str user: Username
95111
:param str repo: Repository
96112
113+
.. note::
114+
Remember :ref:`config precedence`
97115
"""
98116
return self._bool(
99-
self.make_request('pull_requests.merge_status', number=number,
117+
self.make_request('pull_requests.is_merged', number=number,
100118
user=user, repo=repo)
101119
)
102120

103121
def merge(self, number, message='', user=None, repo=None):
104122
"""Merge a pull request.
105123
106124
:param str number: The pull request to merge
125+
:param str message: Message of pull request
107126
:param str user: Username
108127
:param str repo: Repository
109128
129+
.. note::
130+
Remember :ref:`config precedence`
131+
110132
This currently raises an HTTP 405 error if the request is not
111133
mergable.
112134

‎pygithub3/services/pull_requests/comments.py‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
1+
# -*- encoding: utf-8 -*-
12
from pygithub3.services.base import Service, MimeTypeMixin
23

34

45
class Comments(Service, MimeTypeMixin):
56
"""Consume `Review Comments API
6-
<http://developer.github.com/v3/pulls/comments/>`_
7-
8-
"""
7+
<http://developer.github.com/v3/pulls/comments/>`_ """
98

109
def list(self, number, user=None, repo=None):
1110
"""List all the comments for a pull request
1211
1312
:param str number: The number of the pull request
1413
:param str user: Username
1514
:param str repo: Repository
15+
:returns: A :doc:`result`
1616
17+
.. note::
18+
Remember :ref:`config precedence`
1719
"""
1820
return self._get_result(
1921
self.make_request('pull_requests.comments.list', number=number,
@@ -27,37 +29,44 @@ def get(self, number, user=None, repo=None):
2729
:param str user: Username
2830
:param str repo: Repository
2931
32+
.. note::
33+
Remember :ref:`config precedence`
3034
"""
3135
return self._get(
3236
self.make_request('pull_requests.comments.get', number=number,
3337
user=user, repo=repo)
3438
)
3539

36-
def create(self, number, body, user=None, repo=None):
40+
def create(self, number, data, user=None, repo=None):
3741
"""Create a comment
3842
3943
:param str number: the pull request to comment on
44+
:param dict data: Input. See `github pullrequests comments doc`_
4045
:param str user: Username
4146
:param str repo: Repository
4247
48+
.. note::
49+
Remember :ref:`config precedence`
4350
"""
4451
return self._post(
4552
self.make_request('pull_requests.comments.create', number=number,
46-
body=body, user=user, repo=repo)
53+
body=data, user=user, repo=repo)
4754
)
4855

49-
def edit(self, number, body, user=None, repo=None):
56+
def update(self, number, message, user=None, repo=None):
5057
"""Edit a comment
5158
5259
:param str number: The id of the comment to edit
60+
:param str message: Comment message
5361
:param str user: Username
5462
:param str repo: Repository
5563
64+
.. note::
65+
Remember :ref:`config precedence`
5666
"""
57-
return self._patch(
58-
self.make_request('pull_requests.comments.edit', number=number,
59-
body=body, user=user, repo=repo)
60-
)
67+
request = self.make_request('pull_requests.comments.edit',
68+
number=number, body={'body': message}, user=user, repo=repo)
69+
return self._patch(request)
6170

6271
def delete(self, number, user=None, repo=None):
6372
"""Delete a comment
@@ -66,6 +75,8 @@ def delete(self, number, user=None, repo=None):
6675
:param str user: Username
6776
:param str repo: Repository
6877
78+
.. note::
79+
Remember :ref:`config precedence`
6980
"""
7081
return self._delete(
7182
self.make_request('pull_requests.comments.delete', number=number,

‎pygithub3/tests/services/test_pull_requests.py‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77

88
from pygithub3.tests.utils.core import TestCase
99
from pygithub3.services.pull_requests import PullRequests, Comments
10-
from pygithub3.resources.base import json
11-
from pygithub3.requests.base import ValidationError
10+
from pygithub3.requests.base import ValidationError, json
1211
from pygithub3.tests.utils.base import (mock_response, mock_response_result,
1312
mock_json)
1413
from pygithub3.tests.utils.services import _
@@ -107,19 +106,9 @@ def test_LIST_FILES(self, reqm):
107106
('get', _('repos/user/repo/pulls/123/files'))
108107
)
109108

110-
def test_MERGE_STATUS_true(self, reqm):
111-
reqm.return_value = mock_response(204)
112-
resp = self.service.merge_status(123)
113-
self.assertEqual(True, resp)
114-
self.assertEqual(
115-
reqm.call_args[0],
116-
('head', _('repos/user/repo/pulls/123/merge'))
117-
)
118-
119-
def test_MERGE_STATUS_false(self, reqm):
120-
reqm.return_value = mock_response(404)
121-
resp = self.service.merge_status(123)
122-
self.assertEqual(False, resp)
109+
def test_IS_MERGED(self, reqm):
110+
resp = self.service.is_merged(123)
111+
self.assertTrue(resp)
123112
self.assertEqual(
124113
reqm.call_args[0],
125114
('head', _('repos/user/repo/pulls/123/merge'))

0 commit comments

Comments
 (0)