Merge branch 'improve-eng-prod-reviewer-roulette-and-codeowners' into 'master'

Improve frontend codeowners and roulette for Eng. productivity

See merge request gitlab-org/gitlab!19076

.gitlab/CODEOWNERS

@@ -8,6 +8,7 @@
 # Frontend maintainers should see everything in `app/assets/`
 app/assets/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
 *.scss @annabeldunstone @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
+/scripts/frontend/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
 
 # Database maintainers should review changes in `db/`
 db/ @gitlab-org/maintainers/database

lib/gitlab/danger/helper.rb

@@ -93,8 +93,8 @@ module Gitlab
         docs: "~documentation", # Docs are reviewed along DevOps stages, so don't need roulette for now.
         none: "",
         qa: "~QA",
-        test: "~test for `spec/features/*`",
-        engineering_productivity: "Engineering Productivity for CI config review"
+        test: "~test ~Quality for `spec/features/*`",
+        engineering_productivity: '~"Engineering Productivity" for CI, Danger'
       }.freeze
       CATEGORIES = {
         %r{\Adoc/} => :none, # To reinstate roulette for documentation, set to `:docs`.
@@ -104,7 +104,7 @@ module Gitlab
         %r{\A(ee/)?public/} => :frontend,
         %r{\A(ee/)?spec/(javascripts|frontend)/} => :frontend,
         %r{\A(ee/)?vendor/assets/} => :frontend,
-        %r{\Ascripts/frontend/} => :frontend,
+        %r{\A(ee/)?scripts/frontend/} => :frontend,
         %r{(\A|/)(
           \.babelrc |
           \.eslintignore |
@@ -130,14 +130,18 @@ module Gitlab
         %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database,
         %r{\Arubocop/cop/migration(/|\.rb)} => :database,
 
+        %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity,
+        %r{Dangerfile\z} => :engineering_productivity,
+        %r{\A(ee/)?(danger/|lib/gitlab/danger/)} => :engineering_productivity,
+        %r{\A(ee/)?scripts/} => :engineering_productivity,
+
         %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend,
-        %r{\A(ee/)?(bin|config|danger|generator_templates|lib|rubocop|scripts)/} => :backend,
+        %r{\A(ee/)?(bin|config|generator_templates|lib|rubocop)/} => :backend,
         %r{\A(ee/)?spec/features/} => :test,
         %r{\A(ee/)?spec/(?!javascripts|frontend)[^/]+} => :backend,
         %r{\A(ee/)?vendor/(?!assets)[^/]+} => :backend,
         %r{\A(ee/)?vendor/(languages\.yml|licenses\.csv)\z} => :backend,
-        %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity,
-        %r{\A(Dangerfile|Gemfile|Gemfile.lock|Procfile|Rakefile)\z} => :backend,
+        %r{\A(Gemfile|Gemfile.lock|Procfile|Rakefile)\z} => :backend,
         %r{\A[A-Z_]+_VERSION\z} => :backend,
         %r{\A\.rubocop(_todo)?\.yml\z} => :backend,
 

lib/gitlab/danger/teammate.rb

@@ -67,7 +67,10 @@ module Gitlab
 
           area && labels.any?("devops::#{area.downcase}") if kind == :reviewer
         when :engineering_productivity
-          role[/Engineering Productivity/] if kind == :reviewer
+          return false unless role[/Engineering Productivity/]
+          return true if kind == :reviewer
+
+          capabilities(project).include?("#{kind} backend")
         else
           capabilities(project).include?("#{kind} #{category}")
         end

spec/lib/gitlab/danger/helper_spec.rb

@@ -178,6 +178,7 @@ describe Gitlab::Danger::Helper do
       'app/assets/foo'       | :frontend
       'app/views/foo'        | :frontend
       'public/foo'           | :frontend
+      'scripts/frontend/foo' | :frontend
       'spec/javascripts/foo' | :frontend
       'spec/frontend/bar'    | :frontend
       'vendor/assets/foo'    | :frontend
@@ -193,10 +194,8 @@ describe Gitlab::Danger::Helper do
       'app/models/foo' | :backend
       'bin/foo'        | :backend
       'config/foo'     | :backend
-      'danger/foo'     | :backend
       'lib/foo'        | :backend
       'rubocop/foo'    | :backend
-      'scripts/foo'    | :backend
       'spec/foo'       | :backend
       'spec/foo/bar'   | :backend
 
@@ -209,16 +208,24 @@ describe Gitlab::Danger::Helper do
       'vendor/languages.yml'    | :backend
       'vendor/licenses.csv'     | :backend
 
-      'Dangerfile'     | :backend
       'Gemfile'        | :backend
       'Gemfile.lock'   | :backend
       'Procfile'       | :backend
       'Rakefile'       | :backend
       'FOO_VERSION'    | :backend
 
+      'Dangerfile'                                            | :engineering_productivity
+      'danger/commit_messages/Dangerfile'                     | :engineering_productivity
+      'ee/danger/commit_messages/Dangerfile'                  | :engineering_productivity
+      'danger/commit_messages/'                               | :engineering_productivity
+      'ee/danger/commit_messages/'                            | :engineering_productivity
       '.gitlab-ci.yml'                                        | :engineering_productivity
       '.gitlab/ci/cng.gitlab-ci.yml'                          | :engineering_productivity
       '.gitlab/ci/ee-specific-checks.gitlab-ci.yml'           | :engineering_productivity
+      'scripts/foo'                                           | :engineering_productivity
+      'lib/gitlab/danger/foo'                                 | :engineering_productivity
+      'ee/lib/gitlab/danger/foo'                              | :engineering_productivity
+
       'lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml'   | :backend
 
       'ee/FOO_VERSION' | :unknown

spec/lib/gitlab/danger/teammate_spec.rb

@@ -30,7 +30,7 @@ describe Gitlab::Danger::Teammate do
       expect(subject.maintainer?(project, :frontend, labels)).to be_truthy
     end
 
-    context 'when labels contain Create and the category is test' do
+    context 'when labels contain devops::create and the category is test' do
       let(:labels) { ['devops::create'] }
 
       context 'when role is Test Automation Engineer, Create' do
@@ -79,6 +79,22 @@ describe Gitlab::Danger::Teammate do
         it '#maintainer? returns false' do
           expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_falsey
         end
+
+        context 'when capabilities include maintainer backend' do
+          let(:capabilities) { ['maintainer backend'] }
+
+          it '#maintainer? returns true' do
+            expect(subject.maintainer?(project, :engineering_productivity, labels)).to be_truthy
+          end
+        end
+
+        context 'when capabilities include trainee_maintainer backend' do
+          let(:capabilities) { ['trainee_maintainer backend'] }
+
+          it '#traintainer? returns true' do
+            expect(subject.traintainer?(project, :engineering_productivity, labels)).to be_truthy
+          end
+        end
       end
     end
   end