-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add mindev ruletype init
to kick off a rule type
#5228
base: main
Are you sure you want to change the base?
Conversation
213adf2
to
daa24a2
Compare
for _, file := range files { | ||
if _, err := os.Stat(file); err == nil { | ||
|
||
return fmt.Errorf("file %s already exists", file) |
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.
Not sure if you want to check os.IsExist(err)
here before returning this error -- there could be a different reason the stat fails, like a missing parent directory.
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.
I'm not sure how to check this other than trying to create the files or directories.
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.
Oh, I mis-read the code (thought it was err != nil
), and thought you were trying to call os.Create
and then returning an error if the create failed.
return initCmd | ||
} | ||
|
||
func initCmdRun(cmd *cobra.Command, args []string) 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.
What do you think about having this operate in a more "wizard" style in terms of prompting for missing data rather than people needing to figure out the flag arguments up front and then getting a file with a bunch of comments to fill in?
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.
not sure I see much value in that. There is one argument to figure out.
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 wizard-type mode (or extra command-line flags) could fill out the commented sections of the template. For example, if we had a display_name
, we could automatically create the RuleName
based on the display name. We could also prompt for the critical fields like description
, short_failure_message
, and in_entity
.
0623770
to
e0037ba
Compare
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.
Approving, but also explaining the wizard idea a bit more clearly.
I'm also still wondering whether Rego rule types should be able to have a top-level .rego
file, so that it can work with language servers like Regal.
for _, file := range files { | ||
if _, err := os.Stat(file); err == nil { | ||
|
||
return fmt.Errorf("file %s already exists", file) |
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.
Oh, I mis-read the code (thought it was err != nil
), and thought you were trying to call os.Create
and then returning an error if the create failed.
return initCmd | ||
} | ||
|
||
func initCmdRun(cmd *cobra.Command, args []string) 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 wizard-type mode (or extra command-line flags) could fill out the commented sections of the template. For example, if we had a display_name
, we could automatically create the RuleName
based on the display name. We could also prompt for the critical fields like description
, short_failure_message
, and in_entity
.
This helps folks set up the basic skeleton for ruletype writing. Signed-off-by: Juan Antonio Osorio <[email protected]>
Co-authored-by: Evan Anderson <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
e0037ba
to
bf35130
Compare
Summary
This helps folks set up the basic skeleton for ruletype writing.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: