-
-
Notifications
You must be signed in to change notification settings - Fork 598
Fixed the API calls for watching and unwatching a repo #183
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
Thank you :) . But method names should be change as well but old methods proxies to new ones. Also please fixed test and documentation into your PR and it will be perfect :) . |
@Nek- the github UI still talks abotu watching, so it may be fine to keep |
@aaronbloomfield please update the tests as well |
@stof disagree with you. Github keep them only to keep BC. Our methods will fail as soon as they think it's time to remove theses entries. To be sure to provide data as expected, IMO we should set a proxy to the new method. Why do you think that it's better to keep old entry point ? |
(sorry for the delay on this) I've added a subscribe() and unsubscribe() method, while keeping the watch() and unwatch() method; the latter two have been marked in the comments as deprecated. This should allow existing code to continue to use watch() and unwatch(). The tests were all updated (and ones added for subscribe() and unsubscribe()), and TravisCI is happy! |
👍 Thanks
|
} | ||
|
||
/** | ||
* Make the authenticated user unwatch a repository | ||
* @link http://developer.github.com/v3/repos/watching/ | ||
* @deprecated The new command is unsubscribe(), not unwatch() |
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 would be better to write it as @deprecated use unsubscribe() instead
, to match common deprecation messages
fixed af276bc |
As per https://developer.github.com/v3/activity/watching/, it seems the "watched" part of the URL to handle watching and unwatching a repo should be "subscriptions" instead. The updated URL (in this pull request) worked fine for me; the original one didn't.