Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
pkg/utils/file_filter.go
Outdated
| ) | ||
|
|
||
| type FileFilterStrategy interface { | ||
| FilterOut(path string) bool |
There was a problem hiding this comment.
nitpick: I'd call it Filter
pkg/utils/file_filter.go
Outdated
| ignores *gitignore.GitIgnore | ||
| } | ||
|
|
||
| func NewGlobFileFilter(path string, ruleFiles []string, logger *zerolog.Logger) (FileFilterStrategy, error) { |
There was a problem hiding this comment.
I'd call it GitIgnoreFileFilter as it explicitly uses gitignore syntax, I think.
pkg/utils/file_filter.go
Outdated
| logger *zerolog.Logger | ||
| max_threads int64 | ||
| path string | ||
| defaultRules []string |
There was a problem hiding this comment.
this may not be needed anymore, as it could be a strategy, right?
pkg/utils/file_filter.go
Outdated
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| type FileFilterStrategy interface { |
There was a problem hiding this comment.
Potentially more go idiomatic different name (don't care much): Filterable
There was a problem hiding this comment.
Something to consider would be something like this:
func Filter[T any](items []T, predicate func(T) bool) []T {
var result []T
for _, item := range items {
if predicate(item) {
result = append(result, item)
}
}
return result
}
pkg/utils/file_filter.go
Outdated
| if filter.FilterOut(f) { | ||
| keepFile = false | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Yep, exactly like I was thinking how it could/would work.
pkg/utils/file_filter.go
Outdated
| if !globPatternMatcher.MatchesPath(f) { | ||
| // filesToFilter that do not match the filter list are excluded | ||
| keepFile := true | ||
| for _, filter := range fw.filterStrategies { |
There was a problem hiding this comment.
A thought: we could also have a AggregateFileStrategy that delegates to sub-filters and encapsulates the logic.
pkg/utils/file_filter_test.go
Outdated
| b.ResetTimer() | ||
| for n := 0; n < b.N; n++ { | ||
| fileFilter := NewFileFilter(rootDir, &log.Logger, WithThreadNumber(runtime.NumCPU())) | ||
| globFileFilter, err := NewGlobFileFilter(rootDir, ruleFiles, &log.Logger) |
There was a problem hiding this comment.
Probably we should leave that test to ensure that clients that use the NewFileFilter functions right now have a smooth upgrade path.
pkg/utils/file_filter_test.go
Outdated
| globFileFilter, err := NewGlobFileFilter(rootDir, ruleFiles, &log.Logger) | ||
| assert.NoError(b, err) | ||
|
|
||
| fileFilter := NewFileFilter(rootDir, &log.Logger, WithFileFilterStrategies([]FileFilterStrategy{globFileFilter}), WithThreadNumber(runtime.NumCPU())) |
There was a problem hiding this comment.
this should probably be a new test
alina-d-m
force-pushed
the
feat/PS-105/file-filtering
branch
from
f25ae18 to
4426598
Compare
2 participants
No description provided.