-
-
Notifications
You must be signed in to change notification settings - Fork 596
Added 'user/repos' missing endpoint #472
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
Conversation
Nyholm
left a comment
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.
I like the idea but I'm not happy with the option parameter, especially since it's undocumented.
lib/Github/Api/User.php
Outdated
|
|
||
| /** | ||
| * @param null $username | ||
| * @param array $options |
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.
You should explain the valid array keys.
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.
or better add in-code assert
lib/Github/Api/User.php
Outdated
| */ | ||
| public function repos($username = null, array $options = array()) | ||
| { | ||
| $url = $username ? '/users/'.rawurldecode($username).'/repos' : '/user/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.
Hm. This seams like two functions mixed in one method. I'm not 100% convinced. It may be better to "repos()" and "myRepos()" (but with better naming)
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.
There is no other API endpoint that we are using an optional username. I recommend using two functions.
|
@Nyholm I don't agree with you about undocumented options since new options can be added as happened in this case, and the library has to be updated for each change. And also nearly all of the methods in the library can be considered as undocumented; see On the other hand, I think splitting |
Nyholm
left a comment
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.
Okey, fair. Do not document it.
This is the last issue before (I think) we can tag a release candidate.
lib/Github/Api/User.php
Outdated
| } | ||
|
|
||
| /** | ||
| * @deprecated use repos($username, $options) instead |
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.
You should remove this function. It is okey, we will soon release a new major version.
Please make sure to add this in the changelog.
lib/Github/Api/User.php
Outdated
| */ | ||
| public function repos($username = null, array $options = array()) | ||
| { | ||
| $url = $username ? '/users/'.rawurldecode($username).'/repos' : '/user/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.
There is no other API endpoint that we are using an optional username. I recommend using two functions.
lib/Github/Api/User.php
Outdated
| * | ||
| * @return array list of the user repositories | ||
| */ | ||
| public function repos($username = null, array $options = array()) |
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.
you should use short array syntax []
|
@Nyholm @cursedcoder can you review it? |
Nyholm
left a comment
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.
I like it!
List repositories that are accessible to the authenticated user.
https://developer.github.com/v3/repos/#list-your-repositories
On the other hand, I think,
repositories($username, $type = 'owner', $sort = 'full_name', $direction = 'asc')method is outdated sinceaffiliationandvisibilityoptions are not covered