Add repositoryId overloads to methods on I(Observable)RepositoryCommentsClient #1344
Conversation
alexander-efremov
changed the title
| @@ -13,81 +19,157 @@ public interface IObservableRepositoryCommentsClient | |||
| /// <param name="owner">The owner of the repository</param> | |||
| /// <param name="name">The name of the repository</param> | |||
| /// <param name="number">The comment id</param> | |||
| /// <returns></returns> | |||
| /// <returns>A <see cref="IObservable{CommitComment}"/> of <see cref="CommitComment"/> for the specified repository.</returns> | |||
There was a problem hiding this comment.
I think this reads better as "An IObservable ..." rather than "A IObservable ..."
There was a problem hiding this comment.
Rather than call this out everywhere for the file, I'll just come back and check after this comment is folded away
|
@dampir this is looking really good - just a few pointers on the documentation while we're in here updating things. I think once we settle on the reviews for the first couple of these PRs that it should really fly through! |
|
@shiftkey it is looks like I have to review all 26 existent repositoryId PRs to fix XML documentation things, because I've used similar pattern to write docs. So, I'm going through it as slowly and methodically as possible do it 👍 But, firstly, I want add repositoryId PRs for new Reactions clients (they are simple and small 👍 )
I glad to hear that 👍 Let's do this 😄 |
|
@shiftkey I've updated XML docs according your remarks. Please review it whenever possible and if all ok, I'm going to pass through other PRs and update their docs. |
…add-repo-id-repo-comment-client
|
@shiftkey I've cleared XML docs here. I suppose I should clear in every PR that I've done for repository id, because they are similar these ones. |
| /// <remarks>http://developer.github.com/v3/repos/comments/#get-a-single-commit-comment</remarks> | ||
| /// <param name="repositoryId">The ID of the repository</param> | ||
| /// <param name="number">The comment id</param> | ||
| /// <returns></returns> |
There was a problem hiding this comment.
Apologies, I should have been clearer - the whole <returns></returns> element can be removed from these docs.
There was a problem hiding this comment.
@shiftkey done 👍
|
👍 to the additional integration tests here |
As part of my work on #1120 I've added new overloads on I(Observable)RepositoryCommentsClient to get access by repository id.
Update XML documentation of interface methods of clients (also synchronize XML docs of IRepositoryCommentsClient and IObservableRepositoryCommentsClient).
There is some divergence between XML documentation of methods in IRepositoryCommentsClient and IObservableRepositoryCommentsClient. So I've decided
to sync XML documentation of these classes during my work on Add support for interacting with repositories by id. #1120.
Add overloads to IRepositoryCommentsClient.
Just add overloads of existing methods that use repositoryId to work with repo.
Add overloads to IObservableRepositoryCommentsClient.
Just add overloads of existing methods that use repositoryId to work with repo.
Add unit tests.
I've added new unit tests that use repositoryId to work with repo that is just a full copy of existing tests that use (owner, name) key.
Also I've found out that not all methods are covered by tests and added them for new and for old methods.
Add integration tests.
I've added new integration tests that use repositoryId to work with repo that is just a full copy of existing tests that use (owner, name) key.
Also I've found out that not all methods are covered by tests and added them for new and for old methods.
/cc @shiftkey, @ryangribble