diff --git a/Gemfile.lock b/Gemfile.lock index 31dfeec..7b9b916 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - code_ownership (1.36.0) + code_ownership (1.35.0) code_teams (~> 1.0) packs-specification sorbet-runtime (>= 0.5.10821) diff --git a/code_ownership.gemspec b/code_ownership.gemspec index 2d6b4b0..aeeff51 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.0' + spec.version = '1.35.0' 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/ownership_mappers/directory_ownership.rb b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb index 1ac8af9..9bcf26c 100644 --- a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb +++ b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb @@ -10,8 +10,6 @@ class DirectoryOwnership include Mapper CODEOWNERS_DIRECTORY_FILE_NAME = '.codeowner' - RELATIVE_ROOT = Pathname('.').freeze - ABSOLUTE_ROOT = Pathname('/').freeze @@directory_cache = T.let({}, T::Hash[String, T.nilable(CodeTeams::Team)]) # rubocop:disable Style/ClassVars @@ -76,46 +74,36 @@ def owner_for_codeowners_file(codeowners_file) ) end - # Takes a file and finds the relevant `.codeowner` file by walking up the directory + # takes a file and finds the relevant `.codeowner` file by walking up the directory # structure. Example, given `a/b/c.rb`, this looks for `a/b/.codeowner`, `a/.codeowner`, # and `.codeowner` in that order, stopping at the first file to actually exist. - # If the parovided file is a directory, it will look for `.codeowner` in that directory and then upwards. - # We do additional caching so that we don't have to check for file existence every time. + # We do additional caching so that we don't have to check for file existence every time sig { params(file: String).returns(T.nilable(CodeTeams::Team)) } def map_file_to_relevant_owner(file) file_path = Pathname.new(file) - team = T.let(nil, T.nilable(CodeTeams::Team)) + path_components = file_path.each_filename.to_a.map { |path| Pathname.new(path) } - if File.directory?(file) - team = get_team_from_codeowners_file_within_directory(file_path) - end - - while team.nil? && file_path != RELATIVE_ROOT && file_path != ABSOLUTE_ROOT - file_path = file_path.parent - team = get_team_from_codeowners_file_within_directory(file_path) - end - - team - end + (path_components.length - 1).downto(0).each do |i| + potential_relative_path_name = T.must(path_components[0...i]).reduce(Pathname.new('')) { |built_path, path| built_path.join(path) } + potential_codeowners_file = potential_relative_path_name.join(CODEOWNERS_DIRECTORY_FILE_NAME) - sig { params(directory: Pathname).returns(T.nilable(CodeTeams::Team)) } - def get_team_from_codeowners_file_within_directory(directory) - potential_codeowners_file = directory.join(CODEOWNERS_DIRECTORY_FILE_NAME) + potential_codeowners_file_name = potential_codeowners_file.to_s - potential_codeowners_file_name = potential_codeowners_file.to_s + team = nil + if @@directory_cache.key?(potential_codeowners_file_name) + team = @@directory_cache[potential_codeowners_file_name] + elsif potential_codeowners_file.exist? + team = owner_for_codeowners_file(potential_codeowners_file) - team = nil - if @@directory_cache.key?(potential_codeowners_file_name) - team = @@directory_cache[potential_codeowners_file_name] - elsif potential_codeowners_file.exist? - team = owner_for_codeowners_file(potential_codeowners_file) + @@directory_cache[potential_codeowners_file_name] = team + else + @@directory_cache[potential_codeowners_file_name] = nil + end - @@directory_cache[potential_codeowners_file_name] = team - else - @@directory_cache[potential_codeowners_file_name] = nil + return team unless team.nil? end - return team + nil end end end diff --git a/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb index ef1009b..7ef077d 100644 --- a/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb +++ b/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb @@ -21,11 +21,6 @@ module CodeOwnership it 'can find the owner of files in a sub-directory of a team-owned directory' do expect(CodeOwnership.for_file('a/b/c/c_file.jsx').name).to eq 'Bar' end - - it 'looks for codeowner file within directory' do - expect(CodeOwnership.for_file('a/b').name).to eq 'Bar' - expect(CodeOwnership.for_file(Pathname.pwd.join('a/b').to_s).name).to eq 'Bar' - end end end end