diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..be94e6f --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +3.2.2 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