Implement Repository Pages client #1061

M-Zuber · 2016-01-15T11:23:42Z

Fixes #1033
TODO:

💄 usings
Tests: unit
Implementation of the client
IObservables + impl

Questions:

The following is an undocumented endpoint.

 - GET /repos/:owner/:repo/pages/builds:buildID

Should it be included?

The response includes an error field which seems to be mapped to the ApiError class. Does that get included in the model?

Drawing attention

Do enums get their own file?
Is an implementation needed for DebuggerDisplay?
What is the level of importance of integration tests for such a client?

shiftkey · 2016-01-17T23:18:52Z

The following is an undocumented endpoint. ... Should it be included?

Let's stick to things which are documented for now.

The response includes an error field which seems to be mapped to the ApiError class. Does that get included in the model?

I think I'm fine with using ApiError here, but it's hard to say whether there's more properties which aren't displayed in the sample responses included here...

M-Zuber · 2016-01-20T10:55:45Z

I believe I am done, but not sure how to fix the errors re missing files.

cc @shiftkey or @haacked

shiftkey · 2016-01-20T13:29:29Z

not sure how to fix the errors re missing files.

.\build FixProjects should add all files from Octokit.csproj to the cascading projects...

M-Zuber · 2016-01-20T13:40:28Z

Should be done then. Thanks for the help

M-Zuber · 2016-01-21T05:38:43Z

🎉 The errors are not my fault (I think 😄 )
@shiftkey this should be complete for real now

shiftkey · 2016-01-21T06:57:33Z

Holding off on merging this until I can give a better answer on what is happening with #1062

M-Zuber · 2016-01-21T06:58:55Z

👍

shiftkey · 2016-01-25T04:25:30Z

Octokit/Models/Response/ApiError.cs

@@ -9,6 +11,7 @@ namespace Octokit
 #if !NETFX_CORE
    [Serializable]
 #endif
+    [DebuggerDisplay("{DebuggerDisplay,nq}")]


Were these raised because of specific errors with tests?

Since I added the Error Property the conventon test picked it up

Uh, cool? 👍

shiftkey · 2016-01-25T04:36:46Z

Found out why #1062 is occurring and it's an orthogonal discussion to this.

Just a couple of comments about naming things (favouring simpler wherever possible). Everything else looks 💎

shiftkey · 2016-01-25T05:21:29Z

Octokit/Models/Response/PagesBuild.cs

+        /// </summary>
+        public PagesBuildStatus Status { get; protected set; }
+        /// <summary>
+        /// 


Can has some words?

M-Zuber · 2016-01-25T12:11:03Z

@shiftkey it is ready for a re-review

shiftkey · 2016-01-26T03:30:59Z

Octokit.Reactive/Clients/ObservableRepositoryPagesClient.cs

+        /// See the <a href="https://developer.github.com/v3/repos/pages/#list-latest-pages-build">API documentation</a> for more information.
+        /// </remarks>
+        /// <returns></returns>
+        public IObservable<PagesBuild> GetLatestBuild(string owner, string repositoryName)


This will need a rename (and to call the right method on IRepositoryPagesClient)

Sorry I missed that :sad:

shiftkey · 2016-01-26T06:02:46Z

There's also a merge conflict that needs a bit of love here...

…IRepositoryPagesClient

In my defense my machine has been having issues

M-Zuber · 2016-01-26T09:35:30Z

@shiftkey look quick, it is still green 😄

shiftkey · 2016-01-26T09:59:13Z

@M-Zuber quick! merged!

Implement Repository Pages client

M-Zuber · 2016-01-26T10:04:45Z

😆

Thank you for all help. This PR helped me much better understand the structure of octokit.

shiftkey · 2016-01-27T02:10:41Z

Octokit.Tests/Clients/RepositoryPagesClientTests.cs

+
+                client.Get("fake", "repo");
+
+                connection.Received().Get<Page>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/pages"), null);


Not sure how these tests should have passed before the merge, as I managed to break an old PR when I merged master back in. See 5ca6633 for the fix to these tests.

Doh!
I was going to remove that. Should I open a new PR?

Nah, I'll merge that first pass at the documentation into master - so the fix will land there soon.

Add regular interface and models

1a5e3d1

M-Zuber changed the title ~~[WIP] Implement Repository Pages client #1033~~ [WIP] Implement Repository Pages client Jan 16, 2016

Add implementation and unit tests

15c8ff9

M-Zuber force-pushed the IRepositoryPagesClient branch from 912f3ad to 15c8ff9 Compare January 18, 2016 21:20

M-Zuber added 2 commits January 19, 2016 00:15

Figured out how to include the files for the other projects

4265b62

Add IObservable + impl

3003fb3

M-Zuber changed the title ~~[WIP] Implement Repository Pages client~~ Implement Repository Pages client Jan 20, 2016

💄 usings and fix missing files

0435cd2

M-Zuber force-pushed the IRepositoryPagesClient branch from ee3ec4e to 0435cd2 Compare January 20, 2016 13:39

Fix up xml and add missing docs

e690e22

shiftkey mentioned this pull request Jan 21, 2016

How do I test the things #1062

Closed

Fix convention errors

c113292

shiftkey reviewed Jan 25, 2016
View reviewed changes

M-Zuber added 2 commits January 25, 2016 10:26

Add missing words

564dea5

fix merge conflicts

0201d01

shiftkey reviewed Jan 26, 2016
View reviewed changes

shiftkey added the feedback given label Jan 26, 2016

M-Zuber added 3 commits January 26, 2016 10:59

Renaming for 💋

ca187df

Merge branch 'master' of https://github.com/octokit/octokit.net into …

134f44f

…IRepositoryPagesClient

I feel dumb for this one

ef06aa4

In my defense my machine has been having issues

shiftkey added a commit that referenced this pull request Jan 26, 2016

Merge pull request #1061 from M-Zuber/IRepositoryPagesClient

0c4fff0

Implement Repository Pages client

shiftkey merged commit 0c4fff0 into octokit:master Jan 26, 2016

shiftkey reviewed Jan 27, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Repository Pages client #1061

Implement Repository Pages client #1061

M-Zuber commented Jan 15, 2016

shiftkey commented Jan 17, 2016

M-Zuber commented Jan 20, 2016

shiftkey commented Jan 20, 2016

M-Zuber commented Jan 20, 2016

M-Zuber commented Jan 21, 2016

shiftkey commented Jan 21, 2016

M-Zuber commented Jan 21, 2016

shiftkey Jan 25, 2016

M-Zuber Jan 25, 2016

shiftkey Jan 25, 2016

shiftkey commented Jan 25, 2016

shiftkey Jan 25, 2016

M-Zuber commented Jan 25, 2016

shiftkey Jan 26, 2016

M-Zuber Jan 26, 2016

shiftkey commented Jan 26, 2016

M-Zuber commented Jan 26, 2016

shiftkey commented Jan 26, 2016

M-Zuber commented Jan 26, 2016

shiftkey Jan 27, 2016

M-Zuber Jan 27, 2016

shiftkey Jan 27, 2016


		client.Get("fake", "repo");

		connection.Received().Get<Page>(Arg.Is<Uri>(u => u.ToString() == "repos/fake/repo/pages"), null);

Implement Repository Pages client #1061

Implement Repository Pages client #1061

Conversation

M-Zuber commented Jan 15, 2016

shiftkey commented Jan 17, 2016

M-Zuber commented Jan 20, 2016

shiftkey commented Jan 20, 2016

M-Zuber commented Jan 20, 2016

M-Zuber commented Jan 21, 2016

shiftkey commented Jan 21, 2016

M-Zuber commented Jan 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiftkey commented Jan 25, 2016

Choose a reason for hiding this comment

M-Zuber commented Jan 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shiftkey commented Jan 26, 2016

M-Zuber commented Jan 26, 2016

shiftkey commented Jan 26, 2016

M-Zuber commented Jan 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment