Implement Repository Hooks & Repository Forks #313

AndyCross · 2014-01-19T21:07:34Z

Gets the list of hooks defined for a repository

See http://developer.github.com/v3/repos/hooks && http://developer.github.com/v3/repos/forks/ for more information.

Usage:
var hooks = await authedClient.Repository.Hooks.Get("owner", "repo");
var forks = await authedClient.Repository.Forks.Get("owner", "repo");

This also includes the RX assembly.

Resolves #336

shiftkey · 2014-01-19T21:18:52Z

Octokit.Reactive/Clients/IObservableRepositoriesClient.cs

@@ -98,5 +99,14 @@ public interface IObservableRepositoriesClient
 /// that announced this feature.
 /// </remarks>
 IObservableCommitStatusClient CommitStatus { get; }
+
+
+


Clean up the excess whitespace here

shiftkey · 2014-01-19T21:24:29Z

@AndyCross thanks for starting this one off.

One of the design goals we've been aiming for with Octokit is to structure these clients to mirror the Documentation structure (i.e. the sidebar here: http://developer.github.com/v3/)

As you can:

get all hooks
get a single hook
create a hook
edit a hook
delete a hook
test a hook

I think it's worth extracting this code into a IHooksClient (or whatever name makes sense). You don't have to implement all these right now, but it'd make that sample become:

var hooks = await authedClient.Repository.Hooks.GetAll("owner", "repo");

AndyCross · 2014-01-19T22:03:47Z

I thought similar; just wanted to make a small change to get started. I'll work on the basis you suggest.

AndyCross · 2014-01-20T14:39:52Z

I've refactored to a RepositoryClient.Hooks approach; I've also implemented GetHookById and will be adding in other methods (currently throwing NotImplementedException from the client).

Can you feedback on the approach to the RepositoryClient.Hooks while I implement the others?

…rated out - there doesn't need to be this duplicitous 'clarity' given the classes are now separate

AndyCross · 2014-01-20T18:03:36Z

I have added further method implementations;

I have a concern around the TEST method which requires a 0 byte body to be sent it seems; I've implemented this with an empty stub class as the Post body, which works in my production environment but please do review the implementation in terms of style.

shiftkey · 2014-01-20T18:19:40Z

Octokit.Reactive/Clients/ObservableRepositoriesClient.cs

@@ -17,6 +18,8 @@ public ObservableRepositoriesClient(IGitHubClient client)
 _client = client.Repository;
 _connection = client.Connection;
 CommitStatus = new ObservableCommitStatusClient(client);
+ var apiConnection = new ApiConnection(_connection);
+ _hooks = new Lazy<IRepositoryHooksClient>( () => new RepositoryHooksClient(apiConnection));


Any benefits to using Lazy<T> here over just new-ing it up?

Very marginal given how lightweight that ctor is; I was following an example from a Readme method on RepoClient; but that is much more heavyweight. Happy to take your lead and refactor if req.

This one?

octokit.net/Octokit/Models/Response/Readme.cs

Line 25 in 7bfbb72

htmlContent = new Lazy<Task<string>>(async () => await client.GetHtml(HtmlUrl).ConfigureAwait(false));

That's an edge case because we don't want to do the rendering until as late as possible. Let's keep it simple for the moment.

Cool, I'll pull out the lazy loading. Agreed on simplicity.

-----Original Message-----
From: "Brendan Forster" notifications@github.com
Sent: ‎20/‎01/‎2014 19:08
To: "octokit/octokit.net" octokit.net@noreply.github.com
Cc: "Andy Cross" andy@elastacloud.com
Subject: Re: [octokit.net] Implement Repository Hooks (#313)

In Octokit.Reactive/Clients/ObservableRepositoriesClient.cs:

@@ -17,6 +18,8 @@ public ObservableRepositoriesClient(IGitHubClient client)
_client = client.Repository;
_connection = client.Connection;
CommitStatus = new ObservableCommitStatusClient(client);

var apiConnection = new ApiConnection(_connection);

_hooks = new Lazy<IRepositoryHooksClient>( () => new RepositoryHooksClient(apiConnection));
This one?

octokit.net/Octokit/Models/Response/Readme.cs

Line 25 in 7bfbb72

htmlContent = new Lazy<Task<string>>(async () => await client.GetHtml(HtmlUrl).ConfigureAwait(false));

That's an edge case because we don't want to do the rendering until as late as possible. Let's keep it simple for the moment.
—
Reply to this email directly or view it on GitHub.

…RepositoryHooksClient

shiftkey · 2014-01-25T19:19:31Z

[EDIT] added link to relevant issues

shiftkey · 2014-01-25T19:21:17Z

Octokit.Reactive/Clients/IObservableRepositoriesClient.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;


Clean up this unused using statement

shiftkey · 2014-01-25T19:25:20Z

CSC : error CS2001: Source file 'Clients\IRepositoryForksClient.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit.csproj]
CSC : error CS2001: Source file 'Clients\RepositoryForksClient.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit.csproj]
CSC : error CS2001: Source file 'Models\Request\NewRepositoryFork.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit.csproj]
CSC : error CS2001: Source file 'Clients\IRepositoryForksClient.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit-NetCore45.csproj]
CSC : error CS2001: Source file 'Clients\RepositoryForksClient.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit-NetCore45.csproj]
CSC : error CS2001: Source file 'Models\Request\NewRepositoryFork.cs' could not be found [C:\Users\half-ogre\GitHub\qed\qed\bin\Release\.repositories\octokit\octokit.net\Octokit\Octokit-NetCore45.csproj]

Looks like some files are missing from the Forks components...

AndyCross · 2014-02-17T16:31:14Z

Any further work required following git add snafu?

shiftkey · 2014-02-18T03:16:11Z

It's looking good. Unfortunately there's been a lot of change in master since you started this, so they need to be updated before I can merge this in.

AndyCross · 2014-02-18T16:49:39Z

I've added a merge in from the upstream master, it pulled in some mono assembly that I had an issue with file copying (linking?) via the csproj, but that has been resolved now.

Hopefully this is now merge compatible.

haacked · 2014-02-18T17:24:48Z

Hmm, this failed the build. Try running .\build.cmd and see if it adds the Mono files to the right projects. Then commit those changes to your branch.

…udio mono things

AndyCross · 2014-02-20T08:59:48Z

@haacked --c597449 fixes it up; the build.cmd now works locally.

johnduhart · 2014-05-19T19:16:59Z

Can I pick this up @AndyCross? I'd like to get this functionality in.

AndyCross · 2014-05-19T19:31:54Z

Sure!

Sent from my Windows Phone

From: John Du Hartmailto:notifications@github.com
Sent: ý19/ý05/ý2014 20:17
To: octokit/octokit.netmailto:octokit.net@noreply.github.com
Cc: Andy Crossmailto:andy@elastacloud.com
Subject: Re: [octokit.net] Implement Repository Hooks & Repository Forks (#313)

Can I pick this up @AndyCrosshttps://github.com/AndyCross? I'd like to get this functionality in.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/313#issuecomment-43544421.

haacked · 2014-07-01T00:37:30Z

This was picked up by @johnduhart in #495.

AndyCross added 4 commits January 19, 2014 20:36

Implement Get Hooks on Repository Client

528d8c5

Comment missing on Client

8a51438

remove spurious comment from dev

218eeec

Include RX client

8223bad

shiftkey reviewed Jan 19, 2014
View reviewed changes

Refactor to a RepositoryHooksClient

a13c9fa

AndyCross added 7 commits January 20, 2014 15:07

missing test changes & correct tests which were calling Repositories.Get

8222fd9

Change method names to remove [Hooks] reference now that this is sepa…

170b0c4

…rated out - there doesn't need to be this duplicitous 'clarity' given the classes are now separate

implement members except TEST

828b776

complete test method with stub class used for serialization

7cc66aa

missing file

740fd9c

rx client fixup

5f038b8

incorrect namespace

144358b

shiftkey reviewed Jan 20, 2014
View reviewed changes

AndyCross added 3 commits January 21, 2014 07:15

remove references to lazy, correct inappropriate use of non-reactive …

c6f0739

…RepositoryHooksClient

refactor to remove duplication

a70fc80

address code analysis issue

3410d28

hnrkndrssn mentioned this pull request Jan 21, 2014

What's Left To Implement? #246

Closed

Implementation of RepositoryForks api

0c806d7

AndyCross mentioned this pull request Jan 25, 2014

Implement Repository Forks API #336

Closed

2 tasks

shiftkey reviewed Jan 25, 2014
View reviewed changes

missing files

e623632

AndyCross added 2 commits February 18, 2014 16:37

Merge from upstream master

7b54701

add missing files into mono assembly

433f182

fixup csproject files that are not part of the main .sln - xamarin st…

c597449

…udio mono things

johnduhart mentioned this pull request May 20, 2014

[WIP] Implement Repository Hooks & Repository Forks #495

Closed

3 tasks

haacked closed this Jul 1, 2014

kbilsted mentioned this pull request Feb 9, 2015

Forking a repository to current user #698

Closed

nickfloyd added Status: Up for grabs Issues that are ready to be worked on by anyone and removed up-for-grabs labels Oct 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Repository Hooks & Repository Forks #313

Implement Repository Hooks & Repository Forks #313

AndyCross commented Jan 19, 2014

shiftkey Jan 19, 2014

shiftkey commented Jan 19, 2014

AndyCross commented Jan 19, 2014

AndyCross commented Jan 20, 2014

AndyCross commented Jan 20, 2014

shiftkey Jan 20, 2014

AndyCross Jan 20, 2014

shiftkey Jan 20, 2014

AndyCross Jan 20, 2014

shiftkey commented Jan 25, 2014

shiftkey Jan 25, 2014

shiftkey commented Jan 25, 2014

AndyCross commented Feb 17, 2014

shiftkey commented Feb 18, 2014

AndyCross commented Feb 18, 2014

haacked commented Feb 18, 2014

AndyCross commented Feb 20, 2014

johnduhart commented May 19, 2014

AndyCross commented May 19, 2014

haacked commented Jul 1, 2014

		@@ -1,4 +1,5 @@
		using System;
		using System.Collections.Generic;

Implement Repository Hooks & Repository Forks #313

Implement Repository Hooks & Repository Forks #313

Conversation

AndyCross commented Jan 19, 2014

shiftkey Jan 19, 2014

Choose a reason for hiding this comment

shiftkey commented Jan 19, 2014

AndyCross commented Jan 19, 2014

AndyCross commented Jan 20, 2014

AndyCross commented Jan 20, 2014

shiftkey Jan 20, 2014

Choose a reason for hiding this comment

AndyCross Jan 20, 2014

Choose a reason for hiding this comment

shiftkey Jan 20, 2014

Choose a reason for hiding this comment

AndyCross Jan 20, 2014

Choose a reason for hiding this comment

shiftkey commented Jan 25, 2014

shiftkey Jan 25, 2014

Choose a reason for hiding this comment

shiftkey commented Jan 25, 2014

AndyCross commented Feb 17, 2014

shiftkey commented Feb 18, 2014

AndyCross commented Feb 18, 2014

haacked commented Feb 18, 2014

AndyCross commented Feb 20, 2014

johnduhart commented May 19, 2014

AndyCross commented May 19, 2014

haacked commented Jul 1, 2014