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

Added Environments API - GetAll list only feature #2613

Merged
merged 5 commits into from
Nov 17, 2022

Conversation

semyon-p
Copy link
Contributor

Let's add Environment API support.
#2329

As a 1st step I've implemented GetAll methods for Environments.

@nickfloyd
Copy link
Contributor

Hey @semyon-p Thank you so much for the contributions here. If you get the chance, would you mind adding any test coverage that would be appropriate? If needed, you can look at what others have done before you here.

Thank you again for making things better! ❤️

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Overall, things look good here. Just a comment about the tests and also you'll need to add an interface to the Reactive Project - Octokit.Reactive.IObservableRepositoryDeployEnvironmentsClient we keep these in parity with the sync SDK APIs and interfaces. Let me know if you need any help with any of this! Thanks again for the contributions!

@semyon-p
Copy link
Contributor Author

@nickfloyd hi!
I've added tests and IObservableRepositoryDeployEnvironmentsClient.
All tests are passed now. Please check.
I've made a small change in Octokit.Models.Request.ApiOptions class to become such kind of constructions work correctly:
if (options != ApiOptions.None)
{
//do something
}

@semyon-p
Copy link
Contributor Author

@nickfloyd also I wanted to let you know, that I was trying to implement CreateEnvironment method but figured out, that API call described in the documentation does not work for me.
https://docs.github.com/en/rest/deployments/environments#create-or-update-an-environment
I've tried to test on a public personal repository.

DeleteEnvironment call works fine, by the way.
https://docs.github.com/en/rest/deployments/deployments#delete-a-deployment

Maybe I've done something wrong, but if you'll have some time please check it too. Maybe there is a bug in Github API itself.

@nickfloyd
Copy link
Contributor

...also I wanted to let you know, that I was trying to implement CreateEnvironment method but figured out, that API call described in the documentation does not work for me.

Hey @semyon-p thanks for the updates! My guess on Create or Update environment is that since it uses the high REST PUT for both CREATE and UPDATE and does not support POST the verb being used was not the right one. IDK. It seems to work via direct call using PUT. I'll keep looking into it though - It's a good call out and I want to make sure we're not missing anything.

@nickfloyd nickfloyd self-requested a review November 17, 2022 17:49
@mikebollandajw
Copy link

Value was either too large or too small for an Int32. at System.Convert.ThrowInt32OverflowException()
at System.Convert.ToInt32(Int64 value)
at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
at Octokit.PocoJsonSerializerStrategy.DeserializeObject(Object value, Type type) in //Octokit/SimpleJson.cs:line 1437
at Octokit.PocoJsonSerializerStrategy.DeserializeObject(Object value, Type type) in /
/Octokit/SimpleJson.cs:line 1368
at

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants