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

refactor: form to json review #163

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

CarlosRoGuerra
Copy link

@CarlosRoGuerra CarlosRoGuerra commented Jun 14, 2021

Commands:

  • app create
  • app deploy
  • app metadata set
  • app metadata unset
  • app restart
  • app router add
  • app router update
  • app router version add
  • app start
  • app stop
  • app update
  • change password
  • cname add
  • env set
  • event cancel
  • router add
  • router info
  • router list
  • router remove
  • router update
  • service instance add
  • service instance bind
  • service instance update
  • service plan list
  • team create
  • team list
  • team remove
  • token create
  • token delete
  • token info
  • token list
  • token update
  • unit add
  • unit autoscale set
  • unit autoscale unset
  • unit remove
  • unit set
  • user create
  • user info
  • user list
  • volume bind
  • volume create
  • volume update

plans, _, err := apiClient.ServiceApi.ServicePlans(context.Background(), serviceName, &tsuru.ServicePlansOpts{
Pool: optional.NewString(c.pool),
})
plans, _, err := apiClient.ServiceApi.ServicePlans(context.Background(), serviceName, &tsuru.ServicePlansOpts{Pool: optional.NewString(c.pool)})
Copy link
Member

Choose a reason for hiding this comment

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

@CarlosRoGuerra do you prefer one line instead of break lines ?

Copy link
Author

Choose a reason for hiding this comment

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

I prefer it with break line, I think when correcting some things I must have left it that way accidentally

return errors.New("User creation is disabled.")
}
userData := c.userData(ctx, password)
response, err := apiClient.UserApi.UserCreate(context.TODO(), userData)
Copy link
Member

Choose a reason for hiding this comment

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

you should remove context.TODO()

userData := c.userData(ctx, password)
response, err := apiClient.UserApi.UserCreate(context.TODO(), userData)
if err != nil {
return err
Copy link
Member

Choose a reason for hiding this comment

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

why did you remove the "method not allowed" check?

if err != nil {
return err
}
request.Header.Set("Content-Type", "application/x-www-form-urlencoded")
_, err = client.Do(request)
changePassword := c.ChangePassword(old, new, confirm)
Copy link
Member

Choose a reason for hiding this comment

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

this variable name does not make any sense... changePassword->changedPassword or changePassword->newPassword

@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2021

Codecov Report

Merging #163 (93fd277) into master (58fbb0e) will increase coverage by 0.65%.
The diff coverage is 98.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #163      +/-   ##
==========================================
+ Coverage   66.29%   66.94%   +0.65%     
==========================================
  Files          48       48              
  Lines        9011     8962      -49     
==========================================
+ Hits         5974     6000      +26     
+ Misses       2116     2077      -39     
+ Partials      921      885      -36     
Impacted Files Coverage Δ
tsuru/admin/cluster.go 64.35% <0.00%> (-1.81%) ⬇️
tsuru/client/event.go 78.28% <87.50%> (+0.70%) ⬆️
tsuru/client/env.go 70.58% <93.75%> (+2.96%) ⬆️
tsuru/client/auth.go 75.65% <96.87%> (+0.49%) ⬆️
tsuru/client/services.go 71.27% <96.96%> (+1.31%) ⬆️
tsuru/client/apps.go 83.75% <100.00%> (+1.96%) ⬆️
tsuru/client/certificate.go 64.39% <100.00%> (+2.17%) ⬆️
tsuru/client/permission.go 69.66% <100.00%> (+4.00%) ⬆️
tsuru/client/router.go 40.46% <100.00%> (+3.09%) ⬆️
tsuru/client/volume.go 62.45% <100.00%> (+1.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58fbb0e...93fd277. Read the comment docs.

@CarlosRoGuerra CarlosRoGuerra changed the title change: form to json review refactor: form to json review Aug 16, 2021
@CarlosRoGuerra CarlosRoGuerra marked this pull request as ready for review October 4, 2021 17:30
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.

4 participants