Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
WIP add statuses iterator to pull requests #896
Conversation
markddavidoff
force-pushed the
markddavidoff:add-pr-status-iterator
branch
from
54938dc
to
3b5bc2b
|
Also, tests are a must! You can have a look at github3.py/tests/unit/test_pulls.py Lines 261 to 270 in 162f247 PullRequest object, and github3.py/tests/integration/test_pulls.py Lines 39 to 45 in 162f247 |
|
Will add tests! |
markddavidoff
changed the title
| @@ -575,6 +576,21 @@ def reviews(self, number=-1, etag=None): | |||
| url = self._build_url('reviews', base_url=self._api) | |||
| return self._iter(int(number), url, PullReview, etag=etag) | |||
|
|
|||
| def statuses(self, number=-1, etag=None): | |||
| """Iterate over the statuses associated with this pull request. | |||
markddavidoff
Oct 18, 2018
Author
add a .. versionadded:: 1.3.0 to reflect when this function was added to the API.
| :rtype: | ||
| :class:`~github3.repos.Status` | ||
| """ | ||
| return self._iter(int(number), self.statuses_url, status.Status, etag=etag) |
sigmavirus24
Oct 19, 2018
Owner
Everywhere else we build a URL, (search for _build_url in this file for example) but here we're using the URL returned by GitHub. In the past they've been flat out wrong. GitHub recommended to me to just build URLs from scratch. That says to me that we should take a look at what this looks like now, and try to regenerate it from self._api.
markddavidoff
Oct 19, 2018
•
Author
Can you help me understand how _build_url is used pleae?
For the Status endpoint which looks like /repos/:owner/:repo/statuses/:ref, I need the base url /repos/:owner/:repo/ and the ref which i can get from the Head.sha. However, I am unsure how to get that base url within the PullRequest context as self._api there looks like https://api.github.com/repos/octocat/Hello-World/pulls/1347.
I don't see a place where urls are dissected to get a different 'base_url'. This would be useful for #897 which faces the same issue.
sigmavirus24
Oct 19, 2018
Owner
So you could do
self.repository._apiTo get the base url you need, so then you would construct the call like so:
self._build_url('statuses', self.head.sha, base_url=self.repository._api)tl;dr, _build_url takes positional args as parts of the path to be added and base_url is an optional keyword argument that defaults to https://api.github.com unless specified otherwise.
markddavidoff
Oct 19, 2018
•
Author
Thanks for the explanation. I mostly wasn't sure on how to get the base url in this case, so thanks I can see that working.
It builds the repo url from the PullDestination login and name fields using ShortRepository. I think it would be self.repository.respository._api though right (PullRequest.PullDestination.ShortRespository._api)?
It seems roundabout but if we can't always trust the returned URL as you say, then this makes sense.
TODO: build url as in the second example above
sigmavirus24
Oct 20, 2018
Owner
It builds the repo url from the PullDestination login and name fields using ShortRepository. I think it would be self.repository.respository._api though right (PullRequest.PullDestination.ShortRespository._api)?
I was looking at PullDestination's docstring self.base.repository if and only if self.base is actually present, kind of like
github3.py/src/github3/pulls.py
Line 229 in f598e02
markddavidoff
Oct 22, 2018
Author
Nope, you were right idk what i was looking at lol. my bad. I guess I'll return [] in the case there is no base, but I'm confused how that's a valid case?
|
Changed the way the url is built, tests coming soon. |
No description provided.