Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Delete credential flags #774

Merged
merged 16 commits into from
Jan 11, 2021

Conversation

henriquemoraeszup
Copy link
Contributor

Description

Added flag capacity on delete credential

How to verify it

Run the command rit delete credential --provider=<your provider>

Changelog

Added flag capacity to rit delete credential

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>

merge
@henriquemoraeszup henriquemoraeszup force-pushed the hm/delete-credential-flags branch from 35c1293 to e23b3c9 Compare December 14, 2020 11:29
…credential-flags

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>

# Conflicts:
#	pkg/cmd/cmd.go
#	pkg/cmd/delete_credential.go
#	pkg/cmd/delete_credential_test.go
@henriquemoraeszup henriquemoraeszup added 🔨 improvement Improvement in features 🚧 WIP Work in Progress labels Dec 14, 2020
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>

Fixing tests and mocks
@henriquemoraeszup henriquemoraeszup force-pushed the hm/delete-credential-flags branch from d5e8ff4 to 0e9de1b Compare December 14, 2020 17:22
@henriquemoraeszup henriquemoraeszup added ✔️ ready-for-review ready for review and removed 🚧 WIP Work in Progress labels Dec 14, 2020
brunasilvazup
brunasilvazup previously approved these changes Dec 14, 2020
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
…credential-flags

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>

# Conflicts:
#	internal/mocks/mocks.go
@codecov-io
Copy link

codecov-io commented Dec 15, 2020

Codecov Report

Merging #774 (ff08f94) into master (e299f0b) will increase coverage by 0.04%.
The diff coverage is 82.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #774      +/-   ##
==========================================
+ Coverage   83.77%   83.81%   +0.04%     
==========================================
  Files         110      110              
  Lines        3840     3856      +16     
==========================================
+ Hits         3217     3232      +15     
+ Misses        444      443       -1     
- Partials      179      181       +2     
Impacted Files Coverage Δ
pkg/cmd/cmd.go 68.18% <54.54%> (-13.64%) ⬇️
pkg/cmd/delete_credential.go 96.47% <90.90%> (-3.53%) ⬇️
pkg/cmd/formula.go 90.82% <100.00%> (+5.11%) ⬆️
pkg/commands/builder.go 90.30% <100.00%> (+0.04%) ⬆️

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 e299f0b...ff08f94. Read the comment docs.

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
brunasilvazup
brunasilvazup previously approved these changes Dec 15, 2020
pkg/cmd/cmd.go Show resolved Hide resolved
@@ -12,6 +13,15 @@ import (
"github.com/ZupIT/ritchie-cli/pkg/stdin"
)

const (
providerFlagName = "provider"
Copy link
Contributor

Choose a reason for hiding this comment

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

could you extract the flag struct from the formula.go and use addReservedFlags function to generate all flags to any commands?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I am a bit lost, are core commands supposed to have reserved flags?

pkg/cmd/delete_credential.go Show resolved Hide resolved
pkg/cmd/delete_credential.go Show resolved Hide resolved
@henriquemoraeszup henriquemoraeszup added 🚧 WIP Work in Progress and removed ✔️ ready-for-review ready for review 🚧 WIP Work in Progress labels Dec 19, 2020
@henriquemoraeszup henriquemoraeszup added the waiting reply Waiting for an answer to a comment label Dec 19, 2020
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
@henriquemoraeszup henriquemoraeszup removed the waiting reply Waiting for an answer to a comment label Dec 21, 2020
}
prompt.Info(fmt.Sprintf("Current env: %s", env))
func addFlags(flags *pflag.FlagSet) {
flags.String(providerFlagName, "", providerFlagDescription)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can extract this function and struct to cmd.go file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now I see...

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
…credential-flags

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>

# Conflicts:
#	pkg/cmd/formula.go
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
brunasilvazup
brunasilvazup previously approved these changes Jan 4, 2021
Copy link
Contributor

@kaduartur kaduartur left a comment

Choose a reason for hiding this comment

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

Good job 🚀

@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Jan 4, 2021

👌 Merged branch hm/delete-credential-flags into qa

return inputConfig{}, errors.New("you have no defined credentials in this env")
}

providers := make([]string, len(data))
Copy link
Contributor

@kaduartur kaduartur Jan 4, 2021

Choose a reason for hiding this comment

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

When you declare an array like this, you are saying that its capacity is the data size and you are giving each position of the array an empty value, by appending it will add to the next position displaying to the user a select with empty fields.

image

For example, if the data has a size of 3 you will have an array with size 3, and when appending it will add one more position and the size of your array will be 6 with the first 3 empty positions.

we have two ways to fix this:

var providers []string // Simpler

providers := make([]string, 0, len(data)) // More complex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the first option does not pass the linter since we need to preallocate it, going for the second

Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
Signed-off-by: Henrique Moraes <henrique.moraes@zup.com.br>
@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Jan 8, 2021

👌 Merged branch hm/delete-credential-flags into qa

@kaduartur kaduartur merged commit f77fb8d into ZupIT:master Jan 11, 2021
@henriquemoraeszup henriquemoraeszup added the 📚 documentation Improvements or additions to documentation label Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📚 documentation Improvements or additions to documentation 🔨 improvement Improvement in features ✔️ ready-for-review ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests with Testify in Delete credential Support flags for rit delete credential
4 participants