Webhooks, Organisations Administration, Repo migrate, List user repos #7
Conversation
1 similar comment
|
@ethantkoenig what do you say? |
|
@costibleotu Sorry for the inaction, I was AFK last week. I'll try to take a look today or tomorrow. |
|
No problem :). I've just finished another interface, for the Organisations Administration, but I will wait for you to review the Webhooks before doing another PR. When I was doing these interfaces, I thought maybe it would be better to start splitting the interfaces file (is getting bigger and bigger), maybe even the entities one. What do you say? One more thought was: Wouldn't it be nicer to have methods in the entities models, or use the entities as input parameters for the methods in interfaces.py? |
|
Agreed that splitting |
|
LGTM, but can you explain the reason for changing from |
|
The difference between |
|
How should we split the interface? And how should the methods be called afterwords? |
3 similar comments
|
I've updated this PR with all my work so far (Administration Organizations, repo migrate and listing all user's repos). Next PR should be code refactoring based on this discussion. |
costibleotu
changed the title
|
Woah, this is too much change for a single PR. In the future, please issue multiple smaller PRs, it makes reviewing much easier. Also, why are you adding |
test.txt
Outdated
| @@ -0,0 +1 @@ | |||
| asdasdas | |||
There was a problem hiding this comment.
Please remove this file.
gogs_client/entities.py
Outdated
| """ | ||
| Description to the team | ||
|
|
||
| :rtype: int |
gogs_client/entities.py
Outdated
| """ | ||
| Team name | ||
|
|
||
| :rtype: int |
gogs_client/entities.py
Outdated
| """ | ||
| Organization's full name | ||
|
|
||
| :rtype: int |
gogs_client/entities.py
Outdated
| """ | ||
| Organization's username | ||
|
|
||
| :rtype: int |
gogs_client/entities.py
Outdated
| """ | ||
| The events that fires the hook | ||
|
|
||
| :rtype: list of strs |
There was a problem hiding this comment.
List[str] (for consistency, see http://pythonhosted.org/gogs-client/interface.html#gogs_client.interface.GogsApi.get_tokens)
gogs_client/entities.py
Outdated
| @property | ||
| def events(self): | ||
| """ | ||
| The events that fires the hook |
gogs_client/interface.py
Outdated
|
|
||
| :param auth.Authentication auth: authentication object, must be admin-level | ||
| :param str org_name: [Required] Organization user name | ||
| :param str team_name: Full name of the team |
There was a problem hiding this comment.
Argument name is name, but team_name in docstring.
gogs_client/interface.py
Outdated
| :param str team_id: [Required] Id of the team | ||
| :param str username: Username of the user to be added to team | ||
| :return: status code of the request | ||
| :rtype: str |
There was a problem hiding this comment.
In its current form, this method returns a response, not a string.
Similarly for other methods below.
gogs_client/interface.py
Outdated
| :param auth.Authentication auth: authentication object, must be admin-level | ||
| :param str team_id: [Required] Id of the team | ||
| :param str username: Username of the user to be added to team | ||
| :return: status code of the request |
There was a problem hiding this comment.
I don't really like returning the status code, since the point of the gogs_client is to abstract away details. I would either return the newly created team (if possible), or nothing.
Similarly for other methods below.
I've created a Hook entity, and methods for creating, updating and deletion of webhooks.
For the Hook update, I've followed the principle and method for User updates.
I've also modified the methods patch, post and put from http_utils.py (modified "data" param -> "json" - so the gogs-client accepts the requests)
I've added 2 new tests, for the above functions.