Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "feat: check .codeowner within directory if it is provided to for_file" #81

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ruby-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3.2.2
48 changes: 18 additions & 30 deletions lib/code_ownership/private/ownership_mappers/directory_ownership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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