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

Feat add environment client #166

Merged
merged 16 commits into from
Nov 29, 2021
Merged

Feat add environment client #166

merged 16 commits into from
Nov 29, 2021

Conversation

alonnoga
Copy link
Contributor

@alonnoga alonnoga commented Nov 29, 2021

Issue & Steps to Reproduce / Feature Request

adding api client for environments

I'm gonna split the ticket.
first part - basic environment CRUD, with only part of the fields on the resource
second part - rest of the fields ( some field updates require different endpoints such as updateEnvironemntTTL, deployEnvironment etc)

@alonnoga alonnoga self-assigned this Nov 29, 2021
@alonnoga
Copy link
Contributor Author

UserRequiresApproval bool `json:"userRequiresApproval,omitempty"`
Targets string `json:"targets,omitempty"`
PrPlanOnly bool `json:"prPlanOnly,omitempty"`
// TODO: not sure about this one
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure about the type, gonna test it when I add deployments on update

Copy link
Contributor

Choose a reason for hiding this comment

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

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 will change it on the pr for this ticket that way I'll also actually test it


type ConfigurationChangesSchema struct {
Type string `json:"type,omitempty"`
// TODO: not sure this works
Copy link
Contributor Author

Choose a reason for hiding this comment

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

like the one above (this is part of deployment request)

Copy link
Contributor

Choose a reason for hiding this comment

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

this one looks like the same object in api.d.ts so I think it's ok and should work.

type PartialJSONSchema7 = { type?: 'string'; enum?: string[] };

Copy link
Member

@eranelbaz eranelbaz left a comment

Choose a reason for hiding this comment

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

Looking good
just lots of question regrading the types

client/model.go Show resolved Hide resolved
client/model.go Outdated Show resolved Hide resolved
client/model.go Outdated Show resolved Hide resolved
// TODO: not sure about this one
CustomEnv0EnvironmentVariables *interface{} `json:"customEnv0EnvironmentVariables,omitempty"`
GitUserData *GitUserData `json:"gitUserData,omitempty"`
TriggerName string `json:"triggerName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

not for this pr but this one should be User

Copy link
Contributor Author

@alonnoga alonnoga Nov 29, 2021

Choose a reason for hiding this comment

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

you mean on the resource? if so it will be added here

Copy link
Member

Choose a reason for hiding this comment

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

the value of TriggerName should be User

client/model.go Outdated Show resolved Hide resolved
client/environment_test.go Outdated Show resolved Hide resolved
@alonnoga alonnoga merged commit 20ddc79 into main Nov 29, 2021
@alonnoga alonnoga deleted the feat-add-environment-client branch November 29, 2021 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants