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

Add special sorting for special characters in globs #109

Merged
merged 2 commits into from
Nov 21, 2024
Merged
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
2 changes: 1 addition & 1 deletion code_ownership.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |spec|
spec.name = 'code_ownership'
spec.version = '1.38.0'
spec.version = '1.38.1'
spec.authors = ['Gusto Engineers']
spec.email = ['[email protected]']
spec.summary = 'A gem to help engineering teams declare ownership of code'
Expand Down
21 changes: 20 additions & 1 deletion lib/code_ownership/private/codeowners_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,26 @@ def self.expected_contents_lines

next if ownership_entries.none?

codeowners_file_lines += ['', "# #{mapper_description}", *ownership_entries.sort]
# When we have a special character at the beginning of a folder name, then this character
# may be prioritized over *. However, we want the most specific folder to be listed last
# in the CODEOWNERS file, so we should prioritize any character above an asterisk in the
# same position.
if mapper_description == OwnershipMappers::FileAnnotations::DESCRIPTION
# individually owned files definitely won't have globs so we don't need to do special sorting
sorted_ownership_entries = ownership_entries.sort
else
sorted_ownership_entries = ownership_entries.sort do |entry1, entry2|
if entry2.start_with?(entry1.split('**').first)
-1
elsif entry1.start_with?(entry2.split('**').first)
1
else
entry1 <=> entry2
end
end
end

codeowners_file_lines += ['', "# #{mapper_description}", *sorted_ownership_entries]
end

[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ module CodeOwnership

# Owner in .codeowner
/directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
/packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -62,6 +63,7 @@ module CodeOwnership

# Team YML ownership
/config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
EXPECTED
end

Expand Down Expand Up @@ -90,6 +92,7 @@ module CodeOwnership

# Owner in .codeowner
/directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
/packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -99,6 +102,7 @@ module CodeOwnership

# Team YML ownership
/config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
EXPECTED
end
end
Expand Down Expand Up @@ -126,6 +130,12 @@ module CodeOwnership
# code/file owner is notified. Reference GitHub docs for more details:
# https://help.github.com/en/articles/about-code-owners


# Owner in .codeowner
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Team YML ownership
/config/teams/foo.yml @MyOrg/foo-team
EXPECTED
end
end
Expand Down Expand Up @@ -167,6 +177,7 @@ module CodeOwnership

# Owner in .codeowner
# /directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
# /packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -176,6 +187,7 @@ module CodeOwnership

# Team YML ownership
# /config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
EXPECTED
end
end
Expand Down Expand Up @@ -336,8 +348,10 @@ module CodeOwnership

CODEOWNERS should contain the following lines, but does not:
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
- "/config/teams/foo.yml @MyOrg/foo-team"
- "# Owner in .codeowner"
- "/directory/owner/**/** @MyOrg/bar-team"
- "/directory/owner/(my_folder)/**/** @MyOrg/foo-team"
- "# Owner metadata key in package.yml"
- "/packs/my_other_package/**/** @MyOrg/bar-team"

Expand Down Expand Up @@ -377,6 +391,7 @@ module CodeOwnership

# Owner in .codeowner
/directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
/packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -388,6 +403,7 @@ module CodeOwnership

# Team YML ownership
/config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
CODEOWNERS

expect_any_instance_of(codeowners_validation).to_not receive(:`)
Expand Down Expand Up @@ -443,8 +459,10 @@ module CodeOwnership

CODEOWNERS should contain the following lines, but does not:
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
- "/config/teams/foo.yml @MyOrg/foo-team"
- "# Owner in .codeowner"
- "/directory/owner/**/** @MyOrg/bar-team"
- "/directory/owner/(my_folder)/**/** @MyOrg/foo-team"
- "# Owner metadata key in package.yml"
- "/packs/my_other_package/**/** @MyOrg/bar-team"

Expand Down Expand Up @@ -475,6 +493,7 @@ module CodeOwnership

# Owner in .codeowner
/directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
/packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -488,6 +507,7 @@ module CodeOwnership

# Team YML ownership
/config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
CODEOWNERS

expect_any_instance_of(codeowners_validation).to_not receive(:`)
Expand Down Expand Up @@ -516,6 +536,7 @@ module CodeOwnership

# Owner in .codeowner
/directory/owner/**/** @MyOrg/bar-team
/directory/owner/(my_folder)/**/** @MyOrg/foo-team

# Owner metadata key in package.yml
/packs/my_other_package/**/** @MyOrg/bar-team
Expand All @@ -529,6 +550,7 @@ module CodeOwnership

# Team YML ownership
/config/teams/bar.yml @MyOrg/bar-team
/config/teams/foo.yml @MyOrg/foo-team
CODEOWNERS

expect_any_instance_of(codeowners_validation).to_not receive(:`)
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/code_ownership_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
RSpec.describe CodeOwnership do
# Look at individual validations spec to see other validaions that ship with CodeOwnership
# Look at individual validations spec to see other validations that ship with CodeOwnership
describe '.validate!' do
describe 'teams must exist validation' do
before do
Expand Down
10 changes: 10 additions & 0 deletions spec/support/application_fixtures.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def write_configuration(owned_globs: nil, **kwargs)
Bar
CONTENTS
write_file('directory/owner/some_directory_file.ts')
write_file('directory/owner/(my_folder)/.codeowner', <<~CONTENTS)
Foo
CONTENTS
write_file('directory/owner/(my_folder)/some_other_file.ts')

write_file('frontend/javascripts/packages/my_other_package/package.json', <<~CONTENTS)
{
Expand All @@ -36,6 +40,12 @@ def write_configuration(owned_globs: nil, **kwargs)
CONTENTS
write_file('frontend/javascripts/packages/my_other_package/my_file.jsx')

write_file('config/teams/foo.yml', <<~CONTENTS)
name: Foo
github:
team: '@MyOrg/foo-team'
CONTENTS

write_file('config/teams/bar.yml', <<~CONTENTS)
name: Bar
github:
Expand Down