✨ Path inclusion #606
✨ Path inclusion #606
Conversation
|
Finding a way to test this PR with bundle PR |
pranavgaikwad
force-pushed
the
main
branch
4 times, most recently
from
a1be368
to
af66317
Compare
pranavgaikwad
force-pushed
the
main
branch
2 times, most recently
from
0419291
to
200f2f2
Compare
| // if allowFilePaths is not set, path to a file is converted into a path to its base dir | ||
| func GetIncludedPathsFromConfig(i InitConfig, allowFilePaths bool) []string { | ||
| validatedPaths := []string{} | ||
| if includedPaths, ok := i.ProviderSpecificConfig[IncludedPathsConfigKey].([]interface{}); ok { |
There was a problem hiding this comment.
Do we want to error, or warn if it is not a slice?
Just to make debug of provider config easier?
There was a problem hiding this comment.
@shawn-hurley good point.
| absPath = file | ||
| } | ||
| if !p.isFileIncluded(absPath) { | ||
| continue |
There was a problem hiding this comment.
Potentially add a high verbosity log here to note why it was removed.
|
I am investigating the failing test now, everything is working fine locally, looks like something small. |
There was a problem hiding this comment.
Overall, it looks really good to me; great job!
The only point I have is around the true/false option to use dir when getting a file path, just to understand when a user would want to set that to false.
Also, just a minor point but like 5 or 7 verbosity log when filtering out might be helpful in the future.
| @@ -142,3 +142,30 @@ func MultilineGrep(ctx context.Context, window int, path, pattern string) (int, | |||
|
|
|||
| return -1, scanner.Err() | |||
| } | |||
|
|
|||
| // GetIncludedPathsFromConfig returns validated includedPaths from provider settings | |||
| // if allowFilePaths is not set, path to a file is converted into a path to its base dir | |||
There was a problem hiding this comment.
Can you walk me through the use case for this option to be false?
There was a problem hiding this comment.
@shawn-hurley this doesn't affect users, this is for providers...so with Java, we can only scope the path upto granularity of a package (as opposed to a class or file). So if user inputs /util/File.java we can only filter upto /util. I thought this would be potentially useful elsewhere, but I can push this specific logic into java provider too keeping this lib function generic
pranavgaikwad
force-pushed
the
main
branch
2 times, most recently
from
1c23954
to
82daa7f
Compare
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
Bundle PR: 94