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

Implemented Get / IsMember for teams #449

Merged
merged 6 commits into from
Apr 22, 2014
Merged

Conversation

kzu
Copy link
Contributor

@kzu kzu commented Apr 8, 2014

Issue #331.
Added integration tests and a new attribute that requires a new environment variable with an organization to test.

@@ -9,6 +10,7 @@ public static class Helper
{
var githubUsername = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBUSERNAME");
UserName = githubUsername;
Organization = Environment.GetEnvironmentVariable("OCTOKIT_GITHUBORGANIZATION");
Copy link
Member

Choose a reason for hiding this comment

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

Can you update the CONTRIBUTING.md docs to include this new field? Also, do you want to support running both user and org tests in the same context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. Even for organization tests, you always needs a valid user with ownership of the org.

Team tests don't work for users since it appears only orgs can own teams (just realized of this because I got errors trying with a user, and it's not doable on the website either)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CONTRIBUTING.md doc updated!

@haacked
Copy link
Contributor

haacked commented Apr 16, 2014

Ack, there are conflicts so I can't merge this yet.

kzu added 2 commits April 22, 2014 17:12
Issue octokit#331.
Added integration tests and a new attribute that requires a new environment variable with an organization to test.

Conflicts:
	Octokit.Tests.Integration/Octokit.Tests.Integration.csproj
@kzu
Copy link
Contributor Author

kzu commented Apr 22, 2014

Rebased and fixed remaining conflicts.

I need this merge so that I can continue working on the rest of Teams API ;)

@kzu
Copy link
Contributor Author

kzu commented Apr 22, 2014

don't merge yet. got a build error

@kzu
Copy link
Contributor Author

kzu commented Apr 22, 2014

Now it's ready :)

@haacked
Copy link
Contributor

haacked commented Apr 22, 2014

@kzu would you mind adding the corresponding new methods to IObservableTeamsClient?

@kzu
Copy link
Contributor Author

kzu commented Apr 22, 2014

Done :)

Just noticed how the CI works :)

Also added missing null checks

@haacked
Copy link
Contributor

haacked commented Apr 22, 2014

The CI is manually launched for now since we want don't want to run arbitrary third party code automatically. 😄

@haacked
Copy link
Contributor

haacked commented Apr 22, 2014

The failure seems unrelated to these changes.

@haacked
Copy link
Contributor

haacked commented Apr 22, 2014

haacked added a commit that referenced this pull request Apr 22, 2014
Implemented Get / IsMember for teams
@haacked haacked merged commit 0e54ca8 into octokit:master Apr 22, 2014
@kzu kzu deleted the GetAndIsMember branch April 22, 2014 21:40
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.

3 participants