Skip to content

Commit dccf52a

Browse files
authored
Add directory-based file ownership strategy (#72)
* Add directory-based file ownership * Rename .codeowners to .codeowner * Minor version bump
1 parent 3bab672 commit dccf52a

12 files changed

+196
-8
lines changed

README.md

+12-5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ There is also a [companion VSCode Extension]([url](https://github.com/rubyatscal
1212

1313
There are three ways to declare code ownership using this gem.
1414

15+
### Directory-Based Ownership
16+
Directory based ownership allows for all files in that directory and all its sub-directories to be owned by one team. To define this, add a `.codeowner` file inside that directory with the name of the team as the contents of that file.
17+
```
18+
Team
19+
```
20+
21+
### File-Annotation Based Ownership
22+
File annotations are a last resort if there is no clear home for your code. File annotations go at the top of your file, and look like this:
23+
```ruby
24+
# @team MyTeam
25+
```
26+
1527
### Package-Based Ownership
1628
Package based ownership integrates [`packwerk`](https://github.com/Shopify/packwerk) and has ownership defined per package. To define that all files within a package are owned by one team, configure your `package.yml` like this:
1729
```yml
@@ -38,11 +50,6 @@ owned_globs:
3850
- app/services/stuff_belonging_to_my_team/**/**
3951
- app/controllers/other_stuff_belonging_to_my_team/**/**
4052
```
41-
### File-Annotation Based Ownership
42-
File annotations are a last resort if there is no clear home for your code. File annotations go at the top of your file, and look like this:
43-
```ruby
44-
# @team MyTeam
45-
```
4653

4754
### Javascript Package Ownership
4855
Javascript package based ownership allows you to specify an ownership key in a `package.json`. To use this, configure your `package.json` like this:

code_ownership.gemspec

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Gem::Specification.new do |spec|
22
spec.name = 'code_ownership'
3-
spec.version = '1.33.1'
3+
spec.version = '1.34.0'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['[email protected]']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private.rb

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
require 'code_ownership/private/validations/files_have_unique_owners'
1414
require 'code_ownership/private/ownership_mappers/file_annotations'
1515
require 'code_ownership/private/ownership_mappers/team_globs'
16+
require 'code_ownership/private/ownership_mappers/directory_ownership'
1617
require 'code_ownership/private/ownership_mappers/package_ownership'
1718
require 'code_ownership/private/ownership_mappers/js_package_ownership'
1819
require 'code_ownership/private/ownership_mappers/team_yml_ownership'
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
# frozen_string_literal: true
2+
3+
# typed: true
4+
5+
module CodeOwnership
6+
module Private
7+
module OwnershipMappers
8+
class DirectoryOwnership
9+
extend T::Sig
10+
include Mapper
11+
12+
CODEOWNERS_DIRECTORY_FILE_NAME = '.codeowner'
13+
14+
@@directory_cache = T.let({}, T::Hash[String, T.nilable(CodeTeams::Team)]) # rubocop:disable Style/ClassVars
15+
16+
sig do
17+
override.params(file: String).
18+
returns(T.nilable(::CodeTeams::Team))
19+
end
20+
def map_file_to_owner(file)
21+
map_file_to_relevant_owner(file)
22+
end
23+
24+
sig do
25+
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
26+
end
27+
def update_cache(cache, files)
28+
globs_to_owner(files)
29+
end
30+
31+
#
32+
# Directory ownership ignores the passed in files when generating code owners lines.
33+
# This is because Directory ownership knows that the fastest way to find code owners for directory based ownership
34+
# is to simply iterate over the directories and grab the owner, rather than iterating over each file just to get what directory it is in
35+
# In theory this means that we may generate code owners lines that cover files that are not in the passed in argument,
36+
# but in practice this is not of consequence because in reality we never really want to generate code owners for only a
37+
# subset of files, but rather we want code ownership for all files.
38+
#
39+
sig do
40+
override.params(files: T::Array[String]).
41+
returns(T::Hash[String, ::CodeTeams::Team])
42+
end
43+
def globs_to_owner(files)
44+
# The T.unsafe is because the upstream RBI is wrong for Pathname.glob
45+
T
46+
.unsafe(Pathname)
47+
.glob(File.join('**/', CODEOWNERS_DIRECTORY_FILE_NAME))
48+
.map(&:cleanpath)
49+
.each_with_object({}) do |pathname, res|
50+
owner = owner_for_codeowners_file(pathname)
51+
res[pathname.dirname.cleanpath.join('**/**').to_s] = owner
52+
end
53+
end
54+
55+
sig { override.returns(String) }
56+
def description
57+
'Owner in .codeowner'
58+
end
59+
60+
sig { override.void }
61+
def bust_caches!
62+
@@directory_cache = {} # rubocop:disable Style/ClassVars
63+
end
64+
65+
private
66+
67+
sig { params(codeowners_file: Pathname).returns(CodeTeams::Team) }
68+
def owner_for_codeowners_file(codeowners_file)
69+
raw_owner_value = File.foreach(codeowners_file).first.strip
70+
71+
Private.find_team!(
72+
raw_owner_value,
73+
codeowners_file.to_s
74+
)
75+
end
76+
77+
# takes a file and finds the relevant `.codeowner` file by walking up the directory
78+
# structure. Example, given `a/b/c.rb`, this looks for `a/b/.codeowner`, `a/.codeowner`,
79+
# and `.codeowner` in that order, stopping at the first file to actually exist.
80+
# We do additional caching so that we don't have to check for file existence every time
81+
sig { params(file: String).returns(T.nilable(CodeTeams::Team)) }
82+
def map_file_to_relevant_owner(file)
83+
file_path = Pathname.new(file)
84+
path_components = file_path.each_filename.to_a.map { |path| Pathname.new(path) }
85+
86+
(path_components.length - 1).downto(0).each do |i|
87+
potential_relative_path_name = T.must(path_components[0...i]).reduce(Pathname.new('')) { |built_path, path| built_path.join(path) }
88+
potential_codeowners_file = potential_relative_path_name.join(CODEOWNERS_DIRECTORY_FILE_NAME)
89+
90+
potential_codeowners_file_name = potential_codeowners_file.to_s
91+
92+
team = nil
93+
if @@directory_cache.key?(potential_codeowners_file_name)
94+
team = @@directory_cache[potential_codeowners_file_name]
95+
elsif potential_codeowners_file.exist?
96+
team = owner_for_codeowners_file(potential_codeowners_file)
97+
98+
@@directory_cache[potential_codeowners_file_name] = team
99+
else
100+
@@directory_cache[potential_codeowners_file_name] = nil
101+
end
102+
103+
return team unless team.nil?
104+
end
105+
106+
nil
107+
end
108+
end
109+
end
110+
end
111+
end
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
module CodeOwnership
2+
RSpec.describe Private::OwnershipMappers::JsPackageOwnership do
3+
describe 'CodeOwnershp.for_file' do
4+
before do
5+
write_configuration
6+
7+
write_file('a/b/.codeowner', <<~CONTENTS)
8+
Bar
9+
CONTENTS
10+
write_file('a/b/c/c_file.jsx')
11+
write_file('a/b/b_file.jsx')
12+
write_file('config/teams/bar.yml', <<~CONTENTS)
13+
name: Bar
14+
CONTENTS
15+
end
16+
17+
it 'can find the owner of files in team-owned directory' do
18+
expect(CodeOwnership.for_file('a/b/b_file.jsx').name).to eq 'Bar'
19+
end
20+
21+
it 'can find the owner of files in a sub-directory of a team-owned directory' do
22+
expect(CodeOwnership.for_file('a/b/c/c_file.jsx').name).to eq 'Bar'
23+
end
24+
end
25+
end
26+
end

spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb

+3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ module CodeOwnership
2121
## Team-specific owned globs
2222
This team owns nothing in this category.
2323
24+
## Owner in .codeowner
25+
This team owns nothing in this category.
26+
2427
## Owner metadata key in package.yml
2528
This team owns nothing in this category.
2629

spec/lib/code_ownership/private/ownership_mappers/package_ownership_spec.rb

+3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ module CodeOwnership
3030
## Team-specific owned globs
3131
This team owns nothing in this category.
3232
33+
## Owner in .codeowner
34+
This team owns nothing in this category.
35+
3336
## Owner metadata key in package.yml
3437
- packs/my_other_package/**/**
3538

spec/lib/code_ownership/private/ownership_mappers/team_yml_ownership_spec.rb

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ module CodeOwnership
1717
## Team-specific owned globs
1818
This team owns nothing in this category.
1919
20+
## Owner in .codeowner
21+
This team owns nothing in this category.
22+
2023
## Owner metadata key in package.yml
2124
This team owns nothing in this category.
2225

spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ module CodeOwnership
1717
// @team Bar
1818
CONTENTS
1919

20+
write_file('frontend/javascripts/packages/my_package/.codeowner', <<~CONTENTS)
21+
Bar
22+
CONTENTS
23+
2024
write_file('frontend/javascripts/packages/my_package/package.json', <<~CONTENTS)
2125
{
2226
"name": "@gusto/my_package",
@@ -52,8 +56,8 @@ module CodeOwnership
5256
expect(e.message).to eq <<~EXPECTED.chomp
5357
Code ownership should only be defined for each file in one way. The following files have declared ownership in multiple ways.
5458
55-
- frontend/javascripts/packages/my_package/owned_file.jsx (Annotations at the top of file, Team-specific owned globs, Owner metadata key in package.json)
56-
- frontend/javascripts/packages/my_package/package.json (Team-specific owned globs, Owner metadata key in package.json)
59+
- frontend/javascripts/packages/my_package/owned_file.jsx (Annotations at the top of file, Team-specific owned globs, Owner in .codeowner, Owner metadata key in package.json)
60+
- frontend/javascripts/packages/my_package/package.json (Team-specific owned globs, Owner in .codeowner, Owner metadata key in package.json)
5761
- packs/my_pack/owned_file.rb (Annotations at the top of file, Team-specific owned globs, Owner metadata key in package.yml)
5862
- packs/my_pack/package.yml (Team-specific owned globs, Owner metadata key in package.yml)
5963

spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb

+22
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ module CodeOwnership
5151
/app/services/bar_stuff/** @MyOrg/bar-team
5252
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
5353
54+
# Owner in .codeowner
55+
/directory/owner/**/** @MyOrg/bar-team
56+
5457
# Owner metadata key in package.yml
5558
/packs/my_other_package/**/** @MyOrg/bar-team
5659
@@ -85,6 +88,9 @@ module CodeOwnership
8588
/app/services/bar_stuff/** @MyOrg/bar-team
8689
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
8790
91+
# Owner in .codeowner
92+
/directory/owner/**/** @MyOrg/bar-team
93+
8894
# Owner metadata key in package.yml
8995
/packs/my_other_package/**/** @MyOrg/bar-team
9096
@@ -159,6 +165,9 @@ module CodeOwnership
159165
# /app/services/bar_stuff/** @MyOrg/bar-team
160166
# /frontend/javascripts/bar_stuff/** @MyOrg/bar-team
161167
168+
# Owner in .codeowner
169+
# /directory/owner/**/** @MyOrg/bar-team
170+
162171
# Owner metadata key in package.yml
163172
# /packs/my_other_package/**/** @MyOrg/bar-team
164173
@@ -327,6 +336,8 @@ module CodeOwnership
327336
328337
CODEOWNERS should contain the following lines, but does not:
329338
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
339+
- "# Owner in .codeowner"
340+
- "/directory/owner/**/** @MyOrg/bar-team"
330341
- "# Owner metadata key in package.yml"
331342
- "/packs/my_other_package/**/** @MyOrg/bar-team"
332343
@@ -364,6 +375,9 @@ module CodeOwnership
364375
/app/services/bar_stuff/** @MyOrg/bar-team
365376
/frontend/javascripts/bar_stuff/** @MyOrg/bar-team
366377
378+
# Owner in .codeowner
379+
/directory/owner/**/** @MyOrg/bar-team
380+
367381
# Owner metadata key in package.yml
368382
/packs/my_other_package/**/** @MyOrg/bar-team
369383
@@ -429,6 +443,8 @@ module CodeOwnership
429443
430444
CODEOWNERS should contain the following lines, but does not:
431445
- "/packs/my_pack/owned_file.rb @MyOrg/bar-team"
446+
- "# Owner in .codeowner"
447+
- "/directory/owner/**/** @MyOrg/bar-team"
432448
- "# Owner metadata key in package.yml"
433449
- "/packs/my_other_package/**/** @MyOrg/bar-team"
434450
@@ -457,6 +473,9 @@ module CodeOwnership
457473
/frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
458474
/packs/my_pack/owned_file.rb @MyOrg/bar-team
459475
476+
# Owner in .codeowner
477+
/directory/owner/**/** @MyOrg/bar-team
478+
460479
# Owner metadata key in package.yml
461480
/packs/my_other_package/**/** @MyOrg/bar-team
462481
@@ -495,6 +514,9 @@ module CodeOwnership
495514
/packs/my_pack/owned_file.rb @MyOrg/bar-team
496515
/frontend/javascripts/packages/my_package/owned_file.jsx @MyOrg/bar-team
497516
517+
# Owner in .codeowner
518+
/directory/owner/**/** @MyOrg/bar-team
519+
498520
# Owner metadata key in package.yml
499521
/packs/my_other_package/**/** @MyOrg/bar-team
500522

spec/lib/code_ownership_spec.rb

+3
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@
232232
- app/services/bar_stuff/**
233233
- frontend/javascripts/bar_stuff/**
234234
235+
## Owner in .codeowner
236+
- directory/owner/**/**
237+
235238
## Owner metadata key in package.yml
236239
- packs/my_other_package/**/**
237240

spec/support/application_fixtures.rb

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ def write_configuration(owned_globs: nil, **kwargs)
2222
# @team Bar
2323
CONTENTS
2424

25+
write_file('directory/owner/.codeowner', <<~CONTENTS)
26+
Bar
27+
CONTENTS
28+
write_file('directory/owner/some_directory_file.ts')
29+
2530
write_file('frontend/javascripts/packages/my_other_package/package.json', <<~CONTENTS)
2631
{
2732
"name": "@gusto/my_package",

0 commit comments

Comments
 (0)