diff --git a/code_ownership.gemspec b/code_ownership.gemspec index c78af40..cdf01f5 100644 --- a/code_ownership.gemspec +++ b/code_ownership.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'code_ownership' - spec.version = '1.36.2' + spec.version = '1.36.3' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] spec.summary = 'A gem to help engineering teams declare ownership of code' diff --git a/lib/code_ownership/private/glob_cache.rb b/lib/code_ownership/private/glob_cache.rb index f184f7f..33eef0a 100644 --- a/lib/code_ownership/private/glob_cache.rb +++ b/lib/code_ownership/private/glob_cache.rb @@ -6,8 +6,6 @@ module Private class GlobCache extend T::Sig - EXPANDED_CACHE_FILE_COUNT_THRESHOLD = 100 - MapperDescription = T.type_alias { String } CacheShape = T.type_alias do @@ -36,19 +34,15 @@ def raw_cache_contents sig { params(files: T::Array[String]).returns(FilesByMapper) } def mapper_descriptions_that_map_files(files) - if files.count > EXPANDED_CACHE_FILE_COUNT_THRESHOLD - # When looking at many files, expanding the cache out using Dir.glob and checking for intersections is faster - files_by_mappers = files.to_h { |f| [f, Set.new([])] } + files_by_mappers = files.to_h { |f| [f, Set.new([])] } - files_by_mappers_via_expanded_cache.each do |file, mapper| + files_by_mappers_via_expanded_cache.each do |file, mappers| + mappers.each do |mapper| T.must(files_by_mappers[file]) << mapper if files_by_mappers[file] end - - files_by_mappers - else - # When looking at few files, using File.fnmatch is faster - files_by_mappers_via_file_fnmatch(files) end + + files_by_mappers end private @@ -81,31 +75,6 @@ def files_by_mappers_via_expanded_cache files_by_mappers end end - - sig { params(files: T::Array[String]).returns(FilesByMapper) } - def files_by_mappers_via_file_fnmatch(files) - files_by_mappers = T.let({}, FilesByMapper) - - files.each do |file| - files_by_mappers[file] ||= Set.new([]) - @raw_cache_contents.each do |mapper_description, globs_by_owner| - # As much as I'd like to *not* special case the file annotations mapper, using File.fnmatch? on the thousands of files mapped by the - # file annotations mapper is a lot of unnecessary extra work. - # Therefore we can just check if the file is in the globs directly for file annotations, otherwise use File.fnmatch - if mapper_description == OwnershipMappers::FileAnnotations::DESCRIPTION - files_by_mappers.fetch(file) << mapper_description if globs_by_owner[file] - else - globs_by_owner.each_key do |glob| - if File.fnmatch?(glob, file, File::FNM_PATHNAME | File::FNM_EXTGLOB) - files_by_mappers.fetch(file) << mapper_description - end - end - end - end - end - - files_by_mappers - end end end end diff --git a/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb b/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb index b9711d2..20f8349 100644 --- a/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb +++ b/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb @@ -69,26 +69,6 @@ module CodeOwnership it "ignores the file with multiple ownership if it's not in the files param" do expect { CodeOwnership.validate!(files: ['app/services/some_other_file.rb']) }.to_not raise_error end - - context 'there are > 100 files in validation check' do - let(:files) do - files = [] - - 101.times do |i| - filename = "app/services/some_other_file#{i}.rb" - files << filename - write_file(filename, <<~YML) - # @team Bar - YML - end - - files - end - - it "ignores the file with multiple ownership if it's not in the files param" do - expect { CodeOwnership.validate!(files: files) }.to_not raise_error - end - end end context 'with mutliple directory ownership files' do diff --git a/spec/lib/code_ownership_spec.rb b/spec/lib/code_ownership_spec.rb index ae5b70a..1edbaba 100644 --- a/spec/lib/code_ownership_spec.rb +++ b/spec/lib/code_ownership_spec.rb @@ -27,16 +27,10 @@ context 'file ownership with [] characters' do before do - write_file('app/services/[test]/some_other_file.ts', <<~YML) + write_file('app/services/[test]/some_file.ts', <<~TYPESCRIPT) // @team Bar // Countries - YML - - 100.times do |i| - write_file("app/services/withoutbracket/some_other_file#{i}.ts", <<~YML) - // @team Bar - YML - end + TYPESCRIPT end it 'has no validation errors' do