update the way we glob team files #120
Conversation
| code_team = TeamPlugins::Ownership.for(team) | ||
|
|
||
| Dir.glob(code_team.owned_globs).each do |filename| | ||
| map[filename] = team | ||
| end | ||
|
|
||
| # Remove anything that is unowned | ||
| TeamPlugins::Ownership.for(team).unowned_globs.each do |glob| | ||
| Dir.glob(glob).each do |filename| | ||
| map.delete(filename) | ||
| end | ||
| # Remove anything that is unowned by this team | ||
| Dir.glob(code_team.unowned_globs).each do |filename| | ||
| map.delete(filename) |
There was a problem hiding this comment.
this change will make these globs a bit faster
The main reason I'd expect this is that it only tries recursing through the code base once for owned and once for unowned w/ all the glob patterns, as opposed to recursing once per glob.
It might be possible to improve further by doing one giant glob, and using File#fnmatch? to exclude unowned patterns. benchmark-ips + benchmark-memory is super helpful for comparing multiple implementations
There was a problem hiding this comment.
I went with a slightly different Dir.glob(code_team.owned_globs) - Dir.glob(code_team.unowned_globs) that is used elsewhere for now. It might be worth revisiting for fnmatch, but I'll do it in another commit.
There was a problem hiding this comment.
Eh, I couldn't help it. The ugly part here is that unowned_globs is an array, so at best we have to do the following which, thought it might be faster, looping that for every file just annoys me 😬
unowned_globs.any? { File.fnmatch?(_1, filename) }
martinemde
force-pushed
the
aw/me/unowned-globs-tweaks
branch
from
b8d6b40 to
df27298
Compare
martinemde
force-pushed
the
aw/me/unowned-globs-tweaks
branch
from
df27298 to
171bcd9
Compare
@martinemde says that this change will make these globs a bit faster which is important when you're running this on a large codebase.