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: remove -s flag #126

Merged
merged 5 commits into from
Aug 3, 2024
Merged

refactor: remove -s flag #126

merged 5 commits into from
Aug 3, 2024

Conversation

ahmad-ibra
Copy link
Collaborator

@ahmad-ibra ahmad-ibra commented Aug 2, 2024

Description

The proposal of this PR is to simplify our command API by reducing the number of flags we have and using the same flags in different commands.

Namely, this PR focuses on the --silent and --reconfigure flags. Instead of having both flags available for the NewInstallValidatorCmd and NewConfigureValidatorCmd, we can reduce to just using the --reconfigure flag in both commands.

The implications of these changes are this:

  1. No flags passed in => interactive mode
  2. If only -f is used in either the Install or Configure command => silent mode (ie no TUI prompts)
  3. If -f and -r are used in conjuncture in either the Install or Configure command => reconfigure mode (interactive but with defaults from the config provided to users)
  4. If a user runs the install and configure commands in sequence manually (ie without --check), they'll need to run the check command as follows to update their plugin rules: validator check -f validator.yaml -r

Tests

Ran the following:

  • validator install --check => interactive on both install and configure side
  • validator install --check -f validator.yaml => silent on both install and configure side
  • validator install --check -f validator.yaml -r => reconfigure mode on both install and configure side

@ahmad-ibra ahmad-ibra marked this pull request as ready for review August 2, 2024 23:30
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner August 2, 2024 23:30
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Aug 2, 2024
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 42.10526% with 11 lines in your changes missing coverage. Please review.

Files Patch % Lines
pkg/services/validator/validator_service.go 0.00% 6 Missing ⚠️
pkg/cmd/validator/validator.go 37.50% 3 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
- Coverage   36.96%   36.86%   -0.10%     
==========================================
  Files          44       44              
  Lines        5119     5119              
==========================================
- Hits         1892     1887       -5     
- Misses       2882     2885       +3     
- Partials      345      347       +2     
Files Coverage Δ
cmd/validator.go 79.47% <100.00%> (-0.79%) ⬇️
pkg/config/config.go 66.03% <ø> (ø)
...integration/_validator/testcases/test_validator.go 93.44% <100.00%> (ø)
pkg/cmd/validator/validator.go 54.02% <37.50%> (-0.45%) ⬇️
pkg/services/validator/validator_service.go 43.08% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

@ahmad-ibra ahmad-ibra marked this pull request as draft August 2, 2024 23:50
@ahmad-ibra ahmad-ibra removed the bug Something isn't working label Aug 3, 2024
@ahmad-ibra ahmad-ibra changed the title fix: ensure validator install -s and -r flags are mutually exclusive refactor: Aug 3, 2024
@ahmad-ibra ahmad-ibra changed the title refactor: refactor: remove -s flag Aug 3, 2024
@ahmad-ibra ahmad-ibra marked this pull request as ready for review August 3, 2024 01:56
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. refactoring Refactoring / tech debt and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Aug 3, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Aug 3, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 3, 2024
@TylerGillson TylerGillson merged commit 9373e02 into main Aug 3, 2024
8 checks passed
@TylerGillson TylerGillson deleted the fix/validator-flags branch August 3, 2024 16:51
TylerGillson added a commit that referenced this pull request Aug 6, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](v0.0.6...v0.1.0)
(2024-08-06)


### ⚠ BREAKING CHANGES

* split plugin rule configuration and installation into separate
commands
([#121](#121))

### Features

* add docs command; refactor to use embeddedfs pkg
([#116](#116))
([dbe19c5](dbe19c5))
* read CA certs for network rules, add HTTPFileRules, AMIRules
([#117](#117))
([0c4487f](0c4487f))
* support direct rule evaluation with `validator check --direct`
([#127](#127))
([f1fb0d6](f1fb0d6))


### Docs

* added subcommands docs page
([#110](#110))
([9fa23dc](9fa23dc))


### Dependency Updates

* **deps:** update github.com/validator-labs/validator-plugin-azure
digest to ba947e3
([#134](#134))
([2a1058d](2a1058d))
* **deps:** update github.com/validator-labs/validator-plugin-vsphere
digest to 9b1f05b
([#135](#135))
([253f328](253f328))


### Refactoring

* remove -s flag
([#126](#126))
([9373e02](9373e02))
* simplify helm prompts
([#115](#115))
([8ce75a1](8ce75a1))
* split plugin rule configuration and installation into separate
commands
([#121](#121))
([6eaee77](6eaee77))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer refactoring Refactoring / tech debt size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants