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: new validation rule interface #399

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

mattwelke
Copy link
Member

@mattwelke mattwelke commented Aug 21, 2024

Description

New interface to help with working with rule names in a consistent way across the ecosystem.

Plugins would be required to implement it for easy integration with programs like validatorctl. They can do that by embedding validationrule.ManuallyNamed or validationrule.AutomaticallyNamed into their rule struct and implementing the required methods. If they embed validationrule.AutomaticallyNamed, they don't need to implement SetName. Example:

type MyRule struct {
	validationrule.ManuallyNamed `json:",inline"`

	...
}

They should also assert that their rule implements the interface. Example:

var _ validationrule.Interface = (*MyRule)(nil)

For rules that are manually named, it is expected that only pointers to rules can implement the interface because of the pointer receiver of implementations of SetName. Example:

func (r *MyRule) SetName(name string) { ... }

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Files Patch % Lines
pkg/validationrule/validation_rule.go 0.00% 5 Missing ⚠️
pkg/validationresult/validation_result.go 0.00% 2 Missing ⚠️
@@            Coverage Diff             @@
##             main     #399      +/-   ##
==========================================
- Coverage   54.02%   53.78%   -0.24%     
==========================================
  Files          21       22       +1     
  Lines        1131     1136       +5     
==========================================
  Hits          611      611              
- Misses        452      457       +5     
  Partials       68       68              
Files Coverage Δ
pkg/validationresult/validation_result.go 73.58% <0.00%> (ø)
pkg/validationrule/validation_rule.go 0.00% <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 528e21f...75b3b35. Read the comment docs.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke
Copy link
Member Author

Made some changes after review.

  • Renamed "allows manual name" to "requires name"
  • Kept interface named Interface instead of changing it to ValidationRule to avoid package stutter (validationrule.Interface instead of validationrule.ValidationRule)

@mattwelke mattwelke marked this pull request as ready for review August 22, 2024 16:46
@mattwelke mattwelke requested a review from a team as a code owner August 22, 2024 16:46
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Aug 22, 2024
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 22, 2024
@mattwelke mattwelke merged commit 6b16845 into main Aug 22, 2024
7 of 8 checks passed
@mattwelke mattwelke deleted the refactor/validation-rule-interface branch August 22, 2024 18:03
mattwelke pushed a commit that referenced this pull request Aug 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.7](v0.1.6...v0.1.7)
(2024-08-22)


### Refactoring

* new validation rule interface
([#399](#399))
([6b16845](6b16845))

---
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 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