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

Implement user Administrator Client #1068

Merged
merged 8 commits into from
Feb 1, 2016
Merged

Implement user Administrator Client #1068

merged 8 commits into from
Feb 1, 2016

Conversation

paladique
Copy link

Per, #1030. All implemented tests are passing.

@shiftkey
Copy link
Member

@paladique just some xml-docs changes which are making CI cry currently:

 1) Building ./Octokit.sln failed with exitcode 1. 
  2) CS1572: Clients\ObservableUserAdministrationClient.cs(17,26): XML comment has a param tag for 'client', but there is no parameter by that name 
  3) CS1573: Clients\ObservableUserAdministrationClient.cs(18,77): Parameter 'userAdministrationClient' has no matching param tag in the XML comment for 'ObservableUserAdministrationClient.ObservableUserAdministrationClient(IUserAdministrationClient)' (but other parameters do)

Fixed incorrect comment param tag.
@haacked
Copy link
Contributor

haacked commented Jan 29, 2016

Hmm, looks like RepositoryPagesClientTests is failing but that's unrelated to this change. @shiftkey known issue?

@naveensrinivasan
Copy link

Also the convention test are failing because of observables.

The resulting target order is:
 - ConventionTests
Starting Target: ConventionTests 
mono  /Users/naveen/code/octokit.net/tools/xunit.runner.console/tools/xunit.console.exe "/Users/naveen/code/octokit.net/Octokit.Tests.Conventions/bin/Release/Octokit.Tests.Conventions.dll" -parallel none -html "./testresults/xunit.html" 
xUnit.net console test runner (64-bit .NET 4.0.30319.17020)
Copyright (C) 2015 Outercurve Foundation.

Discovering: Octokit.Tests.Conventions
Discovered:  Octokit.Tests.Conventions
Starting:    Octokit.Tests.Conventions
   Octokit.Tests.Conventions.SyncObservableClients.CheckObservableClients(clientInterface: typeof(Octokit.IUsersClient)) [FAIL]
      Octokit.Tests.Conventions.InterfaceMissingMethodsException : Methods not found on interface IObservableUsersClient which are required:
       - get_Administration
      Stack Trace:
         <filename unknown>(0,0): at Octokit.Tests.Conventions.SyncObservableClients.CheckObservableClients (System.Type clientInterface)
            at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
         <filename unknown>(0,0): at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
Finished:    Octokit.Tests.Conventions

@shiftkey
Copy link
Member

@haacked

Hmm, looks like RepositoryPagesClientTests is failing but that's unrelated to this change

You're right. This somehow slipped through the initial PR passing CI https://github.com/octokit/octokit.net/pull/1061/files#r50933117 - but the fix 5ca6633 is now in master.

Perhaps this branch was created before, in which case a merge or cherry-pick should pull in the fix...

@haacked
Copy link
Contributor

haacked commented Feb 1, 2016

@paladique you could merge master into your branch and push. Or, if you're feeling adventurous, rebase your branch against master and force push to this branch.

@shiftkey
Copy link
Member

shiftkey commented Feb 1, 2016

Great work @paladique, and thanks for the contribution!

shiftkey added a commit that referenced this pull request Feb 1, 2016
Implement user Administrator Client
@shiftkey shiftkey merged commit b47678f into master Feb 1, 2016
@shiftkey shiftkey deleted the Administrator-Client branch February 1, 2016 23:08
@haacked
Copy link
Contributor

haacked commented Feb 1, 2016

thumbs-up-pink-glasses

@paladique
Copy link
Author

🎉

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.

4 participants