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
Add support for getting / replacing topics (#634) #832
Conversation
github/tests/Repository.py
Outdated
self.assertEquals(topic in topic_list, True) | ||
|
||
def testReplaceTopics(self): | ||
self.repo.replace_topics(['github', 'testing']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at some of the other tests, I think no assertion is needed here? Let me know if this needs to be improved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine
@@ -2426,6 +2446,22 @@ def protect_branch(self, branch, enabled, enforcement_level=github.GithubObject. | |||
input=post_parameters | |||
) | |||
|
|||
def replace_topics(self, topics): | |||
""" | |||
:calls: `PUT /repos/:owner/:repo/topics <http://developer.github.com/v3/repos>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
github/Repository.py
Outdated
@@ -2304,6 +2312,18 @@ def get_teams(self): | |||
None | |||
) | |||
|
|||
def get_topics(self): | |||
""" | |||
:calls: `GET /repos/:owner/:repo/topics <http://developer.github.com/v3/repos>`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add the exact link:
https://developer.github.com/v3/repos/#list-all-topics-for-a-repository
github/tests/Repository.py
Outdated
def testGetTopics(self): | ||
topic_list = self.repo.get_topics() | ||
topic = u'github' | ||
self.assertEquals(topic in topic_list, True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertIn
?
github/Repository.py
Outdated
@property | ||
def topics(self): | ||
""" | ||
:type: list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
list of string
c5e8d74
to
1cc3832
Compare
@sfdye Thanks for the review feedback, I think I addressed your points -- let me know how that looks. |
@wyardley Thank you very much for putting this together. Merged! |
* Add support for getting and replacing topics on a repository PyGithub#634 * Add / fix tests, accept list vs dict for replace_topics (PyGithub#634)
This is mostly @peterkline's work, just trying to replace #635 (which hasn't been updated) and adjust for some of the changes requested / discussed there.
This seems to resolves the existing test failure, and I think resolves most of the other feedback in the other PR. It also switches the input to
replace_topics()
to the expected array as discussed. Would appreciate feedback, esp. on the test case I added. Because there's no expected return from the method, I think that test is acceptable as-is? I'm not that familiar with this module's test setup, though, so let me know if I should update anything.I didn't have much luck rebasing, so I just patched, but I did retain original author on the first commit.
cc @sfdye