Update prefetch to match cone patterns#70
Conversation
|
@derrickstolee @jrbriggs FYI - I still need to manually test these changes (and see if there are some unit or functional tests that can be added). |
You have some functional tests to update to the new behavior, and that should be enough at the functional level. Perhaps this is an area that could use unit tests? It is definitely an old area, so would be from before we were doing many unit tests. |
derrickstolee
left a comment
There was a problem hiding this comment.
I started with just a nit, but then got a little deeper into perf stuff. I got a bit excited.
You also have functional tests to update and unit tests to write, but I appreciate your work here! This is super important to user productivity.
jeschu1
left a comment
There was a problem hiding this comment.
Questions on unit testing and comments.
|
@jeschu1 @derrickstolee I've switched to a faster matching algorithm and updated the tests. The PR is ready for another look, thanks! |
- Switch to a better performing matching algorithm - Add unit tests - Update functional tests
dfe576a to
747afce
Compare
derrickstolee
left a comment
There was a problem hiding this comment.
I just gave this a quick test on my machine, and it works great! The functional test updates make sense, and the unit tests really show that this works as intended.
| /// </summary> | ||
| public bool UpdatedWholeTree { get; internal set; } = false; | ||
|
|
||
| // public for unit tests |
There was a problem hiding this comment.
Just curious was InternalsVisibleToAttribute another option?
There was a problem hiding this comment.
We haven't set up internals anywhere in our system of projects (yet). Maybe it's something to think about after @mjcheetham converts the projects to .net 3.0 projects?
jeschu1
left a comment
There was a problem hiding this comment.
Looks good, exellent unit tests here for clarity!
Resolves #62
Updated
ShouldIncludeResultto return true for paths that match the included cones.Previously only descendants of the specified folder(s) would be included. With these changes the immediate children of all levels of the specified folder are also included.
Example
Included folder:
A\B\COld behavior
A\B\Care included.New behavior
A\B\Care included.Aare included.A\Bare included.