Skip to content
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

IRepositoriesClient.RepoCollaborators -> IRepositoriesClient.Collaborator #1040

Merged
merged 1 commit into from
Dec 23, 2015

Conversation

M-Zuber
Copy link
Contributor

@M-Zuber M-Zuber commented Dec 23, 2015

Fixes #1036

Updated.

@shiftkey
Copy link
Member

@M-Zuber I think you can make this change without renaming types - perhaps some extra changes are causing the build failures to creep in...

@M-Zuber
Copy link
Contributor Author

M-Zuber commented Dec 23, 2015

Okay, so the wanted change is just the properties on the RepositoriesClients?
I'll start over in a new branch, if that's okay. A little simpler for me

@shiftkey
Copy link
Member

Okay, so the wanted change is just the properties on the RepositoriesClients?

Correct. Let's keep it simple!

I'll start over in a new branch, if that's okay. A little simpler for me

If you're fine with force-pushing this branch to blow away the current changes, I'm fine with that. Otherwise just open a new PR.

@M-Zuber
Copy link
Contributor Author

M-Zuber commented Dec 23, 2015

Force push is fun 😄
I'll be back

@shiftkey
Copy link
Member

Force push is fun 😄

@M-Zuber M-Zuber changed the title [WIP ]Obsolete IRepoCollaborators and introduce new interface IRepositoriesClient.RepoCollaborators -> IRepositoriesClient.Collaborator Dec 23, 2015
@M-Zuber M-Zuber force-pushed the Oboslete-IRepoCollaborators branch from e8780b2 to 8d76a2f Compare December 23, 2015 15:31
@@ -22,7 +22,10 @@ public ObservableRepositoriesClient(IGitHubClient client)
CommitStatus = new ObservableCommitStatusClient(client);
Hooks = new ObservableRepositoryHooksClient(client);
Forks = new ObservableRepositoryForksClient(client);
#pragma warning disable CS0618 // Type or member is obsolete
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that current consumers won't get null

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@M-Zuber
Copy link
Contributor Author

M-Zuber commented Dec 23, 2015

@shiftkey that gif more then accurately describes what I just went through

CI y u no pas??? EDIT: I think I fixed him

@M-Zuber M-Zuber force-pushed the Oboslete-IRepoCollaborators branch from 8d76a2f to a7a1c01 Compare December 23, 2015 15:58
@@ -25,7 +25,10 @@ public RepositoriesClient(IApiConnection apiConnection) : base(apiConnection)
CommitStatus = new CommitStatusClient(apiConnection);
Hooks = new RepositoryHooksClient(apiConnection);
Forks = new RepositoryForksClient(apiConnection);
#pragma warning disable CS0618 // Type or member is obsolete
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

@M-Zuber M-Zuber force-pushed the Oboslete-IRepoCollaborators branch from a7a1c01 to 10c3b2a Compare December 23, 2015 16:30
@M-Zuber
Copy link
Contributor Author

M-Zuber commented Dec 23, 2015

BTW I noticed that there seems to be no tests for this property. Is this on purpose?

@shiftkey
Copy link
Member

BTW I noticed that there seems to be no tests for this property. Is this on purpose?

No, likely just something that was overlooked. Tracking that in #1045

@shiftkey
Copy link
Member

@M-Zuber thanks!

shiftkey added a commit that referenced this pull request Dec 23, 2015
IRepositoriesClient.RepoCollaborators -> IRepositoriesClient.Collaborator
@shiftkey shiftkey merged commit c4ce114 into octokit:master Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants