Commit 1803d62d authored by Kerri Miller's avatar Kerri Miller Committed by Robert Speicher

Remove sectional_codeowners feature flag

Removes the sectional_codeowners feature flag, which has been default
enabled and live on .com for some time. Removing this flag involved
removing a few tests that were specific to the legacy parsing code, as
even a non-sectional codeowners file will still have a default section
in its parsing result ("codeowners")
parent 1a9c3e9c
...@@ -157,12 +157,8 @@ file.md @group-x @group-x/subgroup-y ...@@ -157,12 +157,8 @@ file.md @group-x @group-x/subgroup-y
### Code Owners Sections **(PREMIUM)** ### Code Owners Sections **(PREMIUM)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12137) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.2. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12137) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.2 behind a feature flag, enabled by default.
> - It's deployed behind a feature flag, enabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42389) in GitLab 13.4.
> - It's enabled on GitLab.com.
> - It can be enabled or disabled per-project.
> - It's recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-code-owner-sections). **(CORE ONLY)**
Code Owner rules can be grouped into named sections. This allows for better Code Owner rules can be grouped into named sections. This allows for better
organization of broader categories of Code Owner rules to be applied. organization of broader categories of Code Owner rules to be applied.
...@@ -224,28 +220,6 @@ the rules for "Groups" and "Documentation" sections: ...@@ -224,28 +220,6 @@ the rules for "Groups" and "Documentation" sections:
![MR widget - Sectional Code Owners](img/sectional_code_owners_v13.2.png) ![MR widget - Sectional Code Owners](img/sectional_code_owners_v13.2.png)
#### Enable or disable Code Owner Sections **(CORE ONLY)**
Sections is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
can opt to disable it for your instance.
To disable it:
```ruby
Feature.disable(:sectional_codeowners)
```
To enable it:
```ruby
Feature.enable(:sectional_codeowners)
```
CAUTION: **Caution:**
Disabling Sections will **not** refresh Code Owner Approval Rules on existing merge requests.
## Example `CODEOWNERS` file ## Example `CODEOWNERS` file
```plaintext ```plaintext
......
...@@ -16,35 +16,25 @@ module Gitlab ...@@ -16,35 +16,25 @@ module Gitlab
@parsed_data ||= get_parsed_data @parsed_data ||= get_parsed_data
end end
# Since an otherwise "empty" CODEOWNERS file will still return a default
# section of "codeowners", a la
#
# {"codeowners"=>{}}
#
# ...we must cycle through all the actual values parsed into each
# section to determine if the file is empty or not.
#
def empty? def empty?
parsed_data.empty? parsed_data.values.all?(&:empty?)
end
def entry_for_path(path)
path = "/#{path}" unless path.start_with?('/')
if sectional_codeowners?
sectional_entry_for_path(path)
else
non_sectional_entry_for_path(path)
end
end end
def path def path
@blob&.path @blob&.path
end end
private def entry_for_path(path)
path = "/#{path}" unless path.start_with?('/')
def non_sectional_entry_for_path(path)
matching_pattern = parsed_data.keys.reverse.detect do |pattern|
path_matches?(pattern, path)
end
matching_pattern ? [parsed_data[matching_pattern].dup] : []
end
def sectional_entry_for_path(path)
matches = [] matches = []
parsed_data.each do |_, section_entries| parsed_data.each do |_, section_entries|
...@@ -67,28 +57,6 @@ module Gitlab ...@@ -67,28 +57,6 @@ module Gitlab
end end
def get_parsed_data def get_parsed_data
if sectional_codeowners?
get_parsed_sectional_data
else
get_parsed_non_sectional_data
end
end
def get_parsed_non_sectional_data
parsed = {}
data.lines.each do |line|
line = line.strip
next if skip?(line) || line.match?(SECTION_HEADER_REGEX)
extract_entry_and_populate_parsed_data(line, parsed)
end
parsed
end
def get_parsed_sectional_data
parsed_sectional_data = {} parsed_sectional_data = {}
canonical_section_name = ::Gitlab::CodeOwners::Entry::DEFAULT_SECTION canonical_section_name = ::Gitlab::CodeOwners::Entry::DEFAULT_SECTION
...@@ -168,12 +136,6 @@ module Gitlab ...@@ -168,12 +136,6 @@ module Gitlab
::File.fnmatch?(pattern, path, flags) ::File.fnmatch?(pattern, path, flags)
end end
def sectional_codeowners?
strong_memoize(:sectional_codeowners_check) do
Feature.enabled?(:sectional_codeowners, @project, default_enabled: true)
end
end
end end
end end
end end
...@@ -14,51 +14,30 @@ RSpec.describe Gitlab::CodeOwners::File do ...@@ -14,51 +14,30 @@ RSpec.describe Gitlab::CodeOwners::File do
subject(:file) { described_class.new(blob) } subject(:file) { described_class.new(blob) }
before do
stub_feature_flags(sectional_codeowners: false)
end
describe '#parsed_data' do describe '#parsed_data' do
def owner_line(pattern) def owner_line(pattern)
file.parsed_data[pattern].owner_line file.parsed_data["codeowners"][pattern].owner_line
end end
context "when CODEOWNERS file contains no sections" do
it 'parses all the required lines' do it 'parses all the required lines' do
expected_patterns = [ expected_patterns = [
'/**/*', '/**/#file_with_pound.rb', '/**/*.rb', '/**/CODEOWNERS', '/**/LICENSE', '/docs/**/*', '/**/*', '/**/#file_with_pound.rb', '/**/*.rb', '/**/CODEOWNERS', '/**/LICENSE', '/docs/**/*',
'/docs/*', '/config/**/*', '/**/lib/**/*', '/**/path with spaces/**/*' '/docs/*', '/config/**/*', '/**/lib/**/*', '/**/path with spaces/**/*'
] ]
expect(file.parsed_data.keys) expect(file.parsed_data["codeowners"].keys)
.to contain_exactly(*expected_patterns) .to contain_exactly(*expected_patterns)
end end
it 'allows usernames and emails' do it 'allows usernames and emails' do
expect(owner_line('/**/LICENSE')).to include('legal', 'janedoe@gitlab.com') expect(owner_line('/**/LICENSE')).to include('legal', 'janedoe@gitlab.com')
end end
context "when CODEOWNERS file contains multiple sections" do
let(:file_content) do
File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
end end
let(:patterns) { ["[Documentation]", "[Database]"] } context "when handling a sectional codeowners file" do
let(:paths) { ["/**/[Documentation]", "/**/[Database]"] }
it "skips section headers when parsing" do
expect(file.parsed_data.keys).not_to include(*paths)
expect(file.parsed_data.values.any? { |e| patterns.include?(e.pattern) }).to be_falsey
expect(file.parsed_data.values.any? { |e| e.owner_line.blank? }).to be_falsey
end
end
context "when feature flag `:sectional_codeowners` is enabled" do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
before do
stub_feature_flags(sectional_codeowners: true)
end
shared_examples_for "creates expected parsed sectional results" do shared_examples_for "creates expected parsed sectional results" do
it "is a hash sorted by sections without duplicates" do it "is a hash sorted by sections without duplicates" do
data = file.parsed_data data = file.parsed_data
...@@ -107,12 +86,6 @@ RSpec.describe Gitlab::CodeOwners::File do ...@@ -107,12 +86,6 @@ RSpec.describe Gitlab::CodeOwners::File do
end end
end end
it "passes the call to #get_parsed_sectional_data" do
expect(file).to receive(:get_parsed_sectional_data)
file.parsed_data
end
it "populates a hash with a single default section" do it "populates a hash with a single default section" do
data = file.parsed_data data = file.parsed_data
...@@ -316,11 +289,6 @@ RSpec.describe Gitlab::CodeOwners::File do ...@@ -316,11 +289,6 @@ RSpec.describe Gitlab::CodeOwners::File do
end end
end end
context "when feature flag `:sectional_codeowners` is enabled" do
before do
stub_feature_flags(sectional_codeowners: true)
end
context "when CODEOWNERS file contains no sections" do context "when CODEOWNERS file contains no sections" do
it_behaves_like "returns expected matches" it_behaves_like "returns expected matches"
end end
...@@ -333,23 +301,4 @@ RSpec.describe Gitlab::CodeOwners::File do ...@@ -333,23 +301,4 @@ RSpec.describe Gitlab::CodeOwners::File do
it_behaves_like "returns expected matches" it_behaves_like "returns expected matches"
end end
end end
context "when feature flag `:sectional_codeowners` is disabled" do
before do
stub_feature_flags(sectional_codeowners: false)
end
context "when CODEOWNERS file contains no sections" do
it_behaves_like "returns expected matches"
end
context "when CODEOWNERS file contains multiple sections" do
let(:file_content) do
File.read(Rails.root.join("ee", "spec", "fixtures", "sectional_codeowners_example"))
end
it_behaves_like "returns expected matches"
end
end
end
end end
...@@ -39,10 +39,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do ...@@ -39,10 +39,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
let(:expected_entry) { Gitlab::CodeOwners::Entry.new('docs/CODEOWNERS', '@owner-1 owner2@gitlab.org @owner-3 @documentation-owner') } let(:expected_entry) { Gitlab::CodeOwners::Entry.new('docs/CODEOWNERS', '@owner-1 owner2@gitlab.org @owner-3 @documentation-owner') }
let(:first_entry) { loader.entries.first } let(:first_entry) { loader.entries.first }
before do
stub_feature_flags(sectional_codeowners: false)
end
it 'returns entries for the matched line' do it 'returns entries for the matched line' do
expect(loader.entries).to contain_exactly(expected_entry) expect(loader.entries).to contain_exactly(expected_entry)
end end
...@@ -134,15 +130,11 @@ RSpec.describe Gitlab::CodeOwners::Loader do ...@@ -134,15 +130,11 @@ RSpec.describe Gitlab::CodeOwners::Loader do
end end
end end
context "when sectional_codeowners is disabled" do context "non-sectional codeowners_content" do
before do
stub_feature_flags(sectional_codeowners: false)
end
it_behaves_like "returns users for passed path" it_behaves_like "returns users for passed path"
end end
context "when sectional_codeowners is enabled" do context "when codeowners_content contains sections" do
let(:codeowner_content) do let(:codeowner_content) do
<<~CODEOWNERS <<~CODEOWNERS
[Documentation] [Documentation]
...@@ -153,10 +145,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do ...@@ -153,10 +145,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
CODEOWNERS CODEOWNERS
end end
before do
stub_feature_flags(sectional_codeowners: true)
end
it_behaves_like "returns users for passed path" it_behaves_like "returns users for passed path"
end end
end end
...@@ -178,10 +166,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do ...@@ -178,10 +166,6 @@ RSpec.describe Gitlab::CodeOwners::Loader do
end end
describe '#empty_code_owners?' do describe '#empty_code_owners?' do
before do
stub_feature_flags(sectional_codeowners: false)
end
context 'when file does not exist' do context 'when file does not exist' do
let(:codeowner_blob) { nil } let(:codeowner_blob) { nil }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment