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

Adding flags, delete confirmation and new tests to delete repo #908

Conversation

fernandobelettizup
Copy link
Contributor

Description

Adding flag support to rit delete repo command. Some of the refactoring done as well in this PR:

  • Added delete confirmation to rit delete repo prompt execution.
  • Added tests to support flags and changed tests to work with a tmp directory instead of only mocks.

How to verify it

  • Run rit delete repo --name=<repo> and check if has been deleted succesfully.
  • Run rit delete repo and check if the delete confirmation works.

Changelog

Added flag support for rit delete repo command

…rit delete repo

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
@fernandobelettizup fernandobelettizup added ✔️ ready-for-review ready for review and removed 🚧 WIP Work in Progress labels Apr 16, 2021
@fernandobelettizup fernandobelettizup self-assigned this Apr 16, 2021
@GuillaumeFalourd
Copy link
Contributor

It looks like to use the Input Flags feature, you removed the stdin implementation for the rit delete repo command.
However, as the stdin feature is only deprecated, shouldn't you maintain it as well for compatibility between versions?

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
@fernandobelettizup fernandobelettizup added 🚧 WIP Work in Progress and removed ✔️ ready-for-review ready for review labels Apr 19, 2021
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
@fernandobelettizup fernandobelettizup added ✔️ ready-for-review ready for review and removed 🚧 WIP Work in Progress labels Apr 19, 2021
Comment on lines +58 to +61
if _, err := os.Stat(repoPath); os.IsNotExist(err) {
return errors.New("no repository with this name")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding tests on the deleter_test.go on this rule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like a great idea, but at the same time it feels like it is already being tested.

deleter_test.go already checks for generic errors inside Delete function. And aiming to check for this specific rule tests should be refactored similarly to what happened to delete_repo_test.go.

On the other hand, on deleter_test.go we assure that any removal error is sent to callers and we certify it on "error to delete repo with wrong name" (delete_repo_test.go line 142).

In this fashion, I am not sure if deleter_test.go should get a refactor issue so this rule could be tested as noted.

Copy link
Contributor

Choose a reason for hiding this comment

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

In any way deleter_test will still have to be refactored, maybe not necessary on this PR, but we should keep in mind

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
@fernandobelettizup fernandobelettizup added the 🚧 WIP Work in Progress label Apr 23, 2021
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
brunasilvazup
brunasilvazup previously approved these changes Apr 26, 2021
Copy link
Contributor

@brunasilvazup brunasilvazup left a comment

Choose a reason for hiding this comment

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

👍🏾

Copy link
Contributor

@GuillaumeFalourd GuillaumeFalourd left a comment

Choose a reason for hiding this comment

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

🚀

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.

Just a simple change!

return cmd
}

func (dr deleteRepoCmd) runFunc() CommandRunnerFunc {
func (dr deleteRepoCmd) runFormula() CommandRunnerFunc {
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 rename the function to runCmd?

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented May 3, 2021

👌 Merged branch improvement/support-flags-for-rit-delete-repo into qa

@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented May 3, 2021

👌 Merged branch improvement/support-flags-for-rit-delete-repo into qa

@kaduartur kaduartur merged commit ec0f223 into ZupIT:master May 3, 2021
maurineimirandazup pushed a commit to maurineimirandazup/ritchie-cli that referenced this pull request May 4, 2021
…#908)

* added support flags, added delete confirmation and changed tests for rit delete repo

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* fix lint

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* removed "Delete repo STDIN" from stdin_integration_test

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin removal, added local name for flag

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* fixing unit tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added nameFlag to error message

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added flag to check if repo must be deleted

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* using missingFlagText method

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* removed existingRepoIsDeleted when value is false from tests struct

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin removed tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin scenarios

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* changed sdtin_feature.json repo url, previous url stopped working

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* chaging test repo tag

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* method renamed runFormula -> runCmd

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>
GuillaumeFalourd pushed a commit that referenced this pull request May 24, 2021
* Support flags for the git update repo command.

* Support flags for the rit update repo command.

* Internationalization Ritchie-cli (#847)

* Create config file to ritchie (#825)

* Create translation

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Create config file

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Remove unused dependency

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix lint

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Improve tests

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Improve rit init command (#828)

* Improve rit init command

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix lint

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix functional tests

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Improve rit init tests

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix lint

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix init for Windows

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Remove comment

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix init message

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix lint

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix tests

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Create translation for init command (#834)

* Add translation for init cmd

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix makefile

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix vendor

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix lint

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Fix read config

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Create translation guide

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Update DEVELOPER_GUIDE.md

* Fix config_test.go

Signed-off-by: Kadu Artur Prussek <kadu.artur@gmail.com>

* Adding flags, delete confirmation and new tests to delete repo (#908)

* added support flags, added delete confirmation and changed tests for rit delete repo

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* fix lint

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* removed "Delete repo STDIN" from stdin_integration_test

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin removal, added local name for flag

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* fixing unit tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added nameFlag to error message

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added flag to check if repo must be deleted

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* using missingFlagText method

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* removed existingRepoIsDeleted when value is false from tests struct

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin removed tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting stdin scenarios

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* changed sdtin_feature.json repo url, previous url stopped working

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* chaging test repo tag

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* method renamed runFormula -> runCmd

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* Feature/add formula rit list formulas (#913)

* added list formulas command, changes to tree and builder

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added input flags support

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* creating test file

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added test cases

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* changed check if repo exists logic to avoid for loop

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* tree checker_test fix

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* additional lint fixes (misspelling + preallocation)

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* misspelling fix

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* lint prealloc fix

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* minor suggestion fixes

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added check for empty tree and case tests, changed logic for repo not found

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* reverting error logic on default tree, errors now in list_formula.go

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* lint: misspelling

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added list formula to api.commands

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* simplified a var declaration

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added missingFlagText, changes to constructor and flag description

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added asserts for printed output

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* ignoring print errors on ALL flag, added warning

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* added test case to check warning

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* changed method name, list repos logic, replacer logic, empty repo warning and tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* simplified printformulas to return warnings, changed tests

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* lint fix -> no error returning from printFormulas, just warnings

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* fixing merge buid conflict

Signed-off-by: fernandobelettizup <fernando.beletti@zup.com.br>

* Removal of the Run FuncEF method that I had created. Standardize methods according to other commands.

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Remove comment

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Adjust in return to perform only an update repo on runCmd.

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Variable renamed with a more complete name.

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Linter Fix, gofmt.

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Fix version update prompt, fix array lenght in update flags

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Change error msg

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Change return 'err' to nil

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* add 'externalRepos', with only remote repositorys

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* change err to nil

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* change err to nil

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* First tests

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* gofmt file

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* small changes and working tests

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* All Tests Working, Need Refactor

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Working Tests with assert

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Last test, invalid repo

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* Change output error to fmt.Errorf

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* remove variable flagAll

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* declaration of success message in just one place

Signed-off-by: maurineimirandazup <maurinei.miranda@zup.com.br>

* addition of the help message

addition of the help message

Co-authored-by: Kadu Artur Prussek <kadu.artur@gmail.com>
Co-authored-by: fernandobelettizup <60020008+fernandobelettizup@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 improvement Improvement in features ✔️ ready-for-review ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests and Support flags for rit delete repo
6 participants