chore: handle include globs #553
Conversation
This update introduces a mechanism that handles the globs specified in the source_files configuration. * It adds a new functionality to include source files matching the specified glob patterns and allows the configuration of these source files. *Additionally, it refactors the ignored file code in the profdata_coverage_file class to streamline the condition checks. At it's most basic, the file is checked to see if it's ignore and, if it isn't and there are source_files defined, tested to see if it should be included in the scan.
| end | ||
| return true if path.end_with?("isn't covered.") | ||
| return true if path.include?("/Xcode.*\.app\/Contents\/Developer\/Platforms") | ||
| return true if path.include?("/SourcePackages/checkouts") |
There was a problem hiding this comment.
Do we want to add these two new paths here? If the user adds something to the include list, will that override this ignore?
Ignoring upstream dependencies should probably be up to the developer adding them to the ignore list and not a default here since many different dependency managers exist and will do this in different ways in different folders.
There was a problem hiding this comment.
That's a good point. I debated with myself over this. I have no problem removing them.
No, ignore overrides source-files. My thought being that it's probably more desirable to be able to define the source-file glob(s) in the config and then winnow out anything that you want ignored.
There was a problem hiding this comment.
@jarrodlombardo-EventBase I did update this to remove the exclusion of /SourcePackages/checkouts, though I left the platform exclusion in place. I can't imagine a scenario in which someone would want to slather something Platforms.
I can remove that as well if you really want, but I think the expected behavior that most people would assume is that SDK files are not slathered.
* Removing these paths based on comments. It does make more sense to allow the individual user to define these in the config.
* Use 'source_files_list' instead of 'source_files' * Remove unnecessary console output * The README has also been updated with additional usage notes.
* Minor spelling update
|
@jarrodlombardo-EventBase can you merge this PR? I don't have the mojo. |
|
@ksuther Can you merge this PR, pretty please? |
I also don't have those permissions. Thankfully Kent took care of it. :) |
This update introduces a mechanism that handles the globs specified in the source_files configuration.
source_filesare excluded.At it's most basic, the file is checked to see if it's ignore and, if it isn't and there are source_files defined, tested to see if it should be included in the scan.