-
Notifications
You must be signed in to change notification settings - Fork 363
Conversation
@Meyazhagan Thank you for raising this PR 🤩 We will review it in the upcoming days |
pkg/fileReader/reader.go
Outdated
var filePaths []string | ||
|
||
excludeRe, err := regexp.Compile(excludePattern) |
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 good, but missing a more explicit validation in:
Line 96 in 5daac43
func (flags *TestCommandFlags) Validate() error { |
the error message can be: "invalid --exclude flag: <THE_ERROR>"
@@ -55,16 +56,22 @@ func CreateFileReader(opts *FileReaderOptions) *FileReader { | |||
return fileReader | |||
} | |||
|
|||
func (fr *FileReader) FilterFiles(paths []string) ([]string, error) { | |||
func (fr *FileReader) FilterFiles(paths []string, excludePattern string) ([]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.
please add some unit tests, I suggest starting with reader_test.go
:)
pkg/fileReader/reader.go
Outdated
var filePaths []string | ||
|
||
excludeRe, err := regexp.Compile(excludePattern) |
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.
just a suggestion :)
excludeRe, err := regexp.Compile(excludePattern) | |
excludeRegex, err := regexp.Compile(excludePattern) |
cmd/test/main.go
Outdated
@@ -262,6 +265,7 @@ func (flags *TestCommandFlags) AddFlags(cmd *cobra.Command) { | |||
|
|||
cmd.Flags().StringVarP(&flags.K8sVersion, "schema-version", "s", "", "Set kubernetes version to validate against. Defaults to 1.24.0") | |||
cmd.Flags().StringVarP(&flags.PolicyName, "policy", "p", "", "Policy name to run against") | |||
cmd.Flags().StringVarP(&flags.ExcludePattern, "exclude", "", "", "Exclude paths pattern") |
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.
cmd.Flags().StringVarP(&flags.ExcludePattern, "exclude", "", "", "Exclude paths pattern") | |
cmd.Flags().StringVarP(&flags.ExcludePattern, "exclude", "", "", "Exclude paths pattern (regex)") |
requested some minor changes, should be pretty straight forward :) |
@royhadad I have made the changes. Take a look ! |
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.
looking good! 🚀🚀🚀
@Meyazhagan |
related issues - #732