diff --git a/code_ownership.gemspec b/code_ownership.gemspec index 626d6e7..58cd881 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.38.0' + spec.version = '1.38.1' 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/codeowners_file.rb b/lib/code_ownership/private/codeowners_file.rb index 959a6c1..8acd3a2 100644 --- a/lib/code_ownership/private/codeowners_file.rb +++ b/lib/code_ownership/private/codeowners_file.rb @@ -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 [ diff --git a/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb b/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb index 905be8d..d048bee 100644 --- a/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb +++ b/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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" @@ -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 @@ -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(:`) @@ -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" @@ -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 @@ -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(:`) @@ -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 @@ -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(:`) diff --git a/spec/lib/code_ownership_spec.rb b/spec/lib/code_ownership_spec.rb index 14227a3..27b7a14 100644 --- a/spec/lib/code_ownership_spec.rb +++ b/spec/lib/code_ownership_spec.rb @@ -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 diff --git a/spec/support/application_fixtures.rb b/spec/support/application_fixtures.rb index 9b553f2..3463bc0 100644 --- a/spec/support/application_fixtures.rb +++ b/spec/support/application_fixtures.rb @@ -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) { @@ -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: