-
Notifications
You must be signed in to change notification settings - Fork 44
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
🎨 Improve the structure of some internal packages #441
Conversation
v.AddRule("Organization", validation.Required) | ||
v.AddRule("Organization", validation.Slug) | ||
v.AddRule("Pipeline", validation.Required) | ||
v.AddRule("Pipeline", validation.Slug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an example of how we can add validation to commands
|
||
// Add artifacts if present | ||
if len(v.Artifacts) > 0 { | ||
sections = append(sections, v.renderArtifacts()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our rendering functions are now split out in to their own files, so this can be cleaner and we can just add more if needed
type Rule interface { | ||
Validate(value interface{}) error | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A Rule
just needs to have a function, Validate
which takes in a value
of any
type and returns an error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, could you fix the failing pipeline so we can go ahead
Changes
scopes
to ourvalidation
packagevalidation
package out for clarity/easy addition of rulesbuild
internalsview
package to our/internal/build
, which contains the individual focuses of thebuild
package