Add overloads to the GetAll methods in the IEventsClient #1240

SamTh3D3v · 2016-04-04T15:58:50Z

Refer to the issue #1153

Add an overload for the GetAll* methods on IEventsClient with the ApiOptions parameter.
Implement the the GetAll* method in the EventsClient class.
Add an overload for the GetAll* methods to the IObservableEventsClient with the ApiOptions parameter.
Implement the GetAll* methods in the ObservableEventsClient class.
Implement the needed tests in the EventsClientTests class.
Implement the required integration tests in the ObservableEventsClientTests class.

…ApiOptions as parameter

…face

…s + implement RequestsCorrectUrlWithApiOptions test

…ents test

ryangribble · 2016-04-05T03:30:15Z

Just wondering if you're aware you've got 2 separate github accounts involved here... @samthedev is what you're logged into github.com as, but all your commits are coming from @UsamTheDev 😀

SamTh3D3v · 2016-04-05T05:43:48Z

@ryangribble Yes, that might be an issue that need to be reported, the thing is that i am sending commits from vs, and even though i am logged in as @samthedev, the commits keep coming as @UsamTheDev (another Github account i've created once for testing purposes )

To fix this mess i am guessing that i need to merge the two accounts, not sure how to do that, you can probably save me the googling time :^3

ryangribble · 2016-04-05T06:01:55Z

not sure about being logged in from VS but its all down to the email address associated with the commits. All I do is have a couple of email addresses (eg work and personal) both associated with my one github account and things work fine that way. Im not sure in your situation with the added complexity of one of those email addresses already belonging to "another" github user (ie your 2econd account). Perhaps if you change that account's email address first, then add both email addresses to your main account. Alternatively, just change your VS settings so the email address is the right one and not worry about the historic commits (although its always nice to have all your commits against your own account so the contributors page is accurate!)

SamTh3D3v · 2016-04-05T06:15:33Z

Yes, changing the email in the vs settings pane did solve the issue, thx... it would be great if i can change the author of the older commits as well, is there an easy way to do that ?

shiftkey · 2016-04-05T11:39:47Z

it would be great if i can change the author of the older commits as well, is there an easy way to do that ?

You should check your email addresses defined on GitHub as well

SamTh3D3v · 2016-04-05T13:44:15Z

@shiftkey Yes that did it (after deleting the other github account )..thx

shiftkey · 2016-04-05T17:23:20Z

Octokit.Tests/Clients/ReleasesClientTests.cs

                await Assert.ThrowsAsync<ArgumentNullException>(() => releasesClient.GetAll("owner", null));
+                await Assert.ThrowsAsync<ArgumentException>(() => releasesClient.GetAll("owner", ""));
                await Assert.ThrowsAsync<ArgumentNullException>(() => releasesClient.GetAll("owner", "name", null));


We should probably check ("", "name", ApiOptions.None) and ("owner", "", ApiOptions.None) as well

I think this one was overlooked with the cleanup

shiftkey · 2016-04-05T17:28:39Z

@samthedev just did a pass over all the files in this PR, mostly focusing on the test changes. Looking good!

… + some clean up

SamTh3D3v · 2016-04-06T15:23:59Z

Octokit.Tests.Integration/Reactive/ObservableEventsClientTests.cs

+                _user = github.User.Current().Result.Login;
+                _organization = github.Organization.GetAllForCurrent().Result.First().Login;
+            }
+


@shiftkey pls, i need some help with the GetAllForAnOrganization Method's IntegrationTests ! since the user must be authenticated and the organisation must be associated with that particular user !

Have you configured your integration test settings with a test user (and test organization that this user is an owner of) created specifically for running the integration tests (so as not to mess up your personal account or rate limits etc)?

Then in integration tests, you can get that configured organization just with Helper.Organization (same as you can get the integration user with Helper.UserName)

@ryangribble thx, i miss configured some of the settings, i will try to fix the failing test then i will report back.

All tests passed.

TheGetAllForRepositoryMethod's tests are still failing for some reason !?

I don't believe the test failure is related to this PR.

Here's what I can see: #1251

shiftkey · 2016-04-07T14:19:50Z

It looks like a .swp file was committed in c32dfbe - are you able to remove that from the history?

SamTh3D3v · 2016-04-07T22:32:21Z

It looks like a .swp file was committed in c32dfbe - are you able to remove that from the history?

done.

shiftkey · 2016-04-10T18:20:11Z

Just one more little bit of cleanup that was missed. I'll verify the tests later this week as I'm on terrible internets right now...

shiftkey · 2016-04-10T19:21:51Z

Oh, and that merge conflict 😢

SamTh3D3v · 2016-04-10T19:52:17Z

Yy, my bad ...I believe that's because i touched ReleasesClientTests in 7f93dad, should i undo the changes on that file ?

shiftkey · 2016-04-10T19:57:57Z

@samthedev up to you - I'd suggest just merging master in again...

SamTh3D3v · 2016-04-10T20:20:33Z

@shiftkey that should do it, right ?

shiftkey · 2016-04-10T20:41:49Z

@samthedev just this one comment https://github.com/octokit/octokit.net/pull/1240/files#r58579094

SamTh3D3v · 2016-04-10T20:56:11Z

@shiftkey done.

shiftkey · 2016-04-10T21:23:36Z

Cross-referencing #1251 again to remind myself about digging into that...

shiftkey · 2016-04-10T21:23:44Z

@samthedev thanks!

SamTh3D3v added 10 commits April 4, 2016 16:29

Add overloads to the GetAll methods in the IEventsClient Interface

736cb05

Implement GetAll* overload methods in the EventClient class to allow …

5fae33a

…ApiOptions as parameter

Fix the failling tests after refactoring the GetAll* methds

20087a8

Add the overload GetAlls methods to the IObservableEventsClient inter…

61c7b5e

…face

Implement the GetAll* overloads in the ObservableEventsClient class

e655b40

Fix the failling tests after refactoring the GetAll* methods

2438fa9

Make sure the ApiOtions arn't null in the newly added overload method…

d2e110a

…s + implement RequestsCorrectUrlWithApiOptions test

Implement unit tests for the EventClient's GetAll* overload methods

c519932

Add missing test cases in the ReleaseClientTest's EnsuresNonNullArgum…

57a8c95

…ents test

Fix the last commit

7f93dad

Add the ObservableEventsClientTests class

6a71bef

shiftkey reviewed Apr 5, 2016
View reviewed changes

SamTh3D3v added 4 commits April 5, 2016 21:33

Add an ArgumentException check to the EnsuresArgumentsNotNull methodS…

c32dfbe

… + some clean up

Events GetAll Method Integration Tests

d6c209c

Implement the GetAll* IntegrationTests

c0f7582

TheGetAllForAnOrganizationMethod Integrations test [WIP]

7f98984

SamTh3D3v reviewed Apr 6, 2016
View reviewed changes

SamTh3D3v added 2 commits April 6, 2016 18:59

Fix a failing integration test

e93b8e7

Proper way to call Organization and UserName in the integration test

ee4b6d6

shiftkey mentioned this pull request Apr 7, 2016

New activity events not being deserialized correctly #1251

Closed

Remove .swp file

ee0ceb6

SamTh3D3v added 3 commits April 10, 2016 21:04

Merge branch 'master'

a9c5e9b

merge

b57904a

Fixe after merging

d8adf5c

Add more checks to EnsuresNonNullArguments

a2b52f0

shiftkey merged commit 4ae6000 into octokit:master Apr 10, 2016

SamTh3D3v deleted the PaginationEventsClient branch April 11, 2016 15:58

shiftkey modified the milestone: Pagination Support Jun 3, 2016

shiftkey added the skip-release-notes label Jun 3, 2016

shiftkey mentioned this pull request Jun 12, 2016

Events pagination #625

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add overloads to the GetAll methods in the IEventsClient #1240

Add overloads to the GetAll methods in the IEventsClient #1240

SamTh3D3v commented Apr 4, 2016 •

edited

Loading

ryangribble commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

ryangribble commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

shiftkey commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

shiftkey Apr 5, 2016

shiftkey Apr 10, 2016

shiftkey commented Apr 5, 2016

SamTh3D3v Apr 6, 2016

ryangribble Apr 6, 2016

SamTh3D3v Apr 6, 2016

SamTh3D3v Apr 6, 2016 •

edited

Loading

SamTh3D3v Apr 6, 2016

shiftkey Apr 7, 2016

shiftkey commented Apr 7, 2016

SamTh3D3v commented Apr 7, 2016

shiftkey commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

shiftkey commented Apr 10, 2016

Add overloads to the GetAll methods in the IEventsClient #1240

Add overloads to the GetAll methods in the IEventsClient #1240

Conversation

SamTh3D3v commented Apr 4, 2016 • edited Loading

ryangribble commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

ryangribble commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

shiftkey commented Apr 5, 2016

SamTh3D3v commented Apr 5, 2016

shiftkey Apr 5, 2016

Choose a reason for hiding this comment

shiftkey Apr 10, 2016

Choose a reason for hiding this comment

shiftkey commented Apr 5, 2016

SamTh3D3v Apr 6, 2016

Choose a reason for hiding this comment

ryangribble Apr 6, 2016

Choose a reason for hiding this comment

SamTh3D3v Apr 6, 2016

Choose a reason for hiding this comment

SamTh3D3v Apr 6, 2016 • edited Loading

Choose a reason for hiding this comment

SamTh3D3v Apr 6, 2016

Choose a reason for hiding this comment

shiftkey Apr 7, 2016

Choose a reason for hiding this comment

shiftkey commented Apr 7, 2016

SamTh3D3v commented Apr 7, 2016

shiftkey commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 10, 2016

shiftkey commented Apr 10, 2016

shiftkey commented Apr 10, 2016

SamTh3D3v commented Apr 4, 2016 •

edited

Loading

SamTh3D3v Apr 6, 2016 •

edited

Loading