Skip to content

refactor: split ConfigureOrCheckCommand into multiple functions#222

Merged
ahmad-ibra merged 3 commits intomainfrom
refactor/split-configure-check-commands
Sep 11, 2024
Merged

refactor: split ConfigureOrCheckCommand into multiple functions#222
ahmad-ibra merged 3 commits intomainfrom
refactor/split-configure-check-commands

Conversation

@ahmad-ibra
Copy link
Member

@ahmad-ibra ahmad-ibra commented Sep 11, 2024

Issue

N/A

Description

Breaks the ConfigureOrCheckCommand function down into a ConfigureCommand function and a CheckCommand function. Also introduces a new configureValidatorConfig helper function.

With this change, the cognitive load of modifying either the ConfigureCommand or CheckCommand functions should be reduced because we no longer need to keep both direct evaluation and continuous evaluation in mind.

@ahmad-ibra ahmad-ibra force-pushed the refactor/split-configure-check-commands branch 4 times, most recently from 5864f71 to dc10ec6 Compare September 11, 2024 17:55
@ahmad-ibra ahmad-ibra force-pushed the refactor/split-configure-check-commands branch from dc10ec6 to 2857ac3 Compare September 11, 2024 17:55
@ahmad-ibra ahmad-ibra marked this pull request as ready for review September 11, 2024 18:03
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner September 11, 2024 18:03
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 11, 2024
@dosubot dosubot bot added the refactoring Refactoring / tech debt label Sep 11, 2024
Copy link
Member

@TylerGillson TylerGillson left a comment

Choose a reason for hiding this comment

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

Sorry... DRY nazi here. Please put:

	ok, invalidPlugins := vc.EnabledPluginsHaveRules()
	if !ok {
		log.FatalCLI("invalid validator configuration", "error",
			fmt.Sprintf("the following plugins are enabled, but have no rules configured: %v", invalidPlugins),
		)
	}

Into a helper.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 11, 2024
@codecov
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 52.50000% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/cmd/validator/validator.go 52.63% 11 Missing and 7 partials ⚠️
cmd/validator.go 50.00% 1 Missing ⚠️
@@           Coverage Diff           @@
##             main     #222   +/-   ##
=======================================
  Coverage   53.30%   53.30%           
=======================================
  Files          46       46           
  Lines        6339     6352   +13     
=======================================
+ Hits         3379     3386    +7     
- Misses       2114     2117    +3     
- Partials      846      849    +3     
Files with missing lines Coverage Δ
cmd/validator.go 82.60% <50.00%> (ø)
pkg/cmd/validator/validator.go 59.97% <52.63%> (-0.10%) ⬇️

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 180f0e3...f58cb1f. Read the comment docs.

@ahmad-ibra ahmad-ibra merged commit 466413a into main Sep 11, 2024
@ahmad-ibra ahmad-ibra deleted the refactor/split-configure-check-commands branch September 11, 2024 18:48
TylerGillson added a commit that referenced this pull request Sep 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.2.1](v0.2.0...v0.2.1)
(2024-09-11)


### Features

* support for Azure quota rule
([#224](#224))
([8e61091](8e61091))


### Bug Fixes

* ignore nil errors in validationResponseOk
([#227](#227))
([26544e2](26544e2))


### Refactoring

* split ConfigureOrCheckCommand into multiple functions
([#222](#222))
([466413a](466413a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
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:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants