Switch directory ownership to git ls-files
#80
Conversation
|
The tests are failing here because the use of |
|
I just remembered why we don't use |
|
Maybe we could add an optional configuration to do this? Could even be something super customizable like a proc configuration that would just do the file glob be default. |
|
@gmacdougall you read my mind 😁 Yeah, detecting the ability to use git ls-files would make sense. If we make it a lambda, could even experiment with some of the fancier tools like |
|
I like that we are trying to improve the speed of this command. I would prefer to find a solution that runs the same code in local development and on CI. |
This switches away from using a glob match on '**' within `DirectoryOwnership` matcher in favour of using `git ls-files`. This provides a significant speed improvement for large repositories for a few reasons. The primary reason is that we're only searching through tracked files and avoiding potentially large directores, such as `node_modules`. The major intended side effect of this is that it only is dealing with tracked files rather than all files. I'm opening the PR for further discussion and refinement to see if this is worth considering. Speed comparison for me locally: Before: ``` ❯ time bin/codeownership validate ________________________________________________________ Executed in 16.72 secs fish external usr time 3.08 secs 0.10 millis 3.08 secs sys time 10.70 secs 1.59 millis 10.70 secs ``` After: ``` ❯ time bin/codeownership validate ________________________________________________________ Executed in 10.67 secs fish external usr time 2.90 secs 111.00 micros 2.90 secs sys time 8.65 secs 741.00 micros 8.65 secs ```
gmacdougall
force-pushed
the
gmacdougall-use-ls-files-in-directory-ownership
branch
from
3e135ec to
a7f1a70
Compare
This allows us to decide which algorithm to use for finding code ownership files within the repo. If the configuration directive is set to use_git_ls_files then we will use that rather than finding files via a glob. This will lead to a significant speed improvement as finding files via glob is slow.
This allows us to configure a tool to use git ls-files over using the standard file glob matching.
gmacdougall
force-pushed
the
gmacdougall-use-ls-files-in-directory-ownership
branch
from
a7f1a70 to
04eea8f
Compare
This switches `-use-git-ls-files` to `--use-git-ls-files` because I screwed this up.
This passes the argument from the CLI onto the configuration to be loaded into the configuration through semi-hacky means.
|
Yeah, this is great. Don't worry about this comment:
The problem is that CI doesn't have everything we have locally. I'm OK with compensating for CI if we don't want to add a checkout step there for performance reasons. |
This switches away from using a glob match on '**' within
DirectoryOwnershipmatcher in favour of usinggit ls-files. Thisprovides a significant speed improvement for large repositories for a
few reasons.
The primary reason is that we're only searching through tracked files
and avoiding potentially large directores, such as
node_modules.The major intended side effect of this is that it only is dealing with
tracked files rather than all files.
I'm opening the PR for further discussion and refinement to see if this
is worth considering.
Speed comparison for me locally:
Before:
After: