diff --git a/keeps/delete_old_feature_flags.rb b/keeps/delete_old_feature_flags.rb
index 928c861add0e75f178f080700e1bf6b3fae23a96..d59fab8133de7891919d5794bba05855782188c1 100644
--- a/keeps/delete_old_feature_flags.rb
+++ b/keeps/delete_old_feature_flags.rb
@@ -5,6 +5,7 @@
require 'httparty'
require 'json'
require_relative './helpers/groups'
+require_relative './helpers/milestones'
module Keeps
# This is an implementation of a ::Gitlab::Housekeeper::Keep. This keep will locate any featrure flag definition file
@@ -14,7 +15,6 @@ module Keeps
#
# ```
# bundle exec gitlab-housekeeper -d \
- # -r keeps/delete_old_feature_flags.rb \
# -k Keeps::DeleteOldFeatureFlags
# ```
# rubocop:disable Gitlab/HTTParty -- Don't use GitLab dependencies
@@ -51,7 +51,9 @@ def each_change
next
end
- next unless before_cuttoff?(feature_flag.milestone)
+ next unless milestones_helper.before_cuttoff?(
+ milestone: feature_flag.milestone,
+ milestones_ago: CUTOFF_MILESTONE_OLD)
# feature_flag_default_enabled = feature_flag_definition[:default_enabled]
# feature_flag_gitlab_com_state = fetch_gitlab_com_state(feature_flag.name)
@@ -60,13 +62,13 @@ def each_change
# # Finalize the migration
change = ::Gitlab::Housekeeper::Change.new
- change.title = "Delete the `#{feature_flag.name}` feature flag introduced in #{feature_flag.milestone}"
-
+ change.changelog_type = 'removed'
+ change.title = "Delete the `#{feature_flag.name}` feature flag"
change.identifiers = [self.class.name.demodulize, feature_flag.name]
# rubocop:disable Gitlab/DocUrl -- Not running inside rails application
change.description = <<~MARKDOWN
- This feature flag was introduced more than #{CUTOFF_MILESTONE_OLD} milestones ago.
+ This feature flag was introduced in #{feature_flag.milestone}, which is more than #{CUTOFF_MILESTONE_OLD} milestones ago.
As part of our process we want to ensure [feature flags don't stay too long in the codebase](https://docs.gitlab.com/ee/development/feature_flags/#types-of-feature-flags).
@@ -77,10 +79,6 @@ def each_change
```
</details>
-
- This merge request was created using the
- [gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139492)
- gem.
MARKDOWN
# rubocop:enable Gitlab/DocUrl
@@ -89,7 +87,8 @@ def each_change
change.changed_files = [feature_flag_yaml_file]
change.labels = [
- 'maintenance::refactor',
+ 'maintenance::removal',
+ 'feature flag',
feature_flag.group
]
@@ -97,19 +96,14 @@ def each_change
if change.reviewers.empty?
group_data = groups_helper.group_for_group_label(feature_flag.group)
- if group_data
- change.reviewers = groups_helper.pick_reviewer(group_data, change.identifiers)
- end
+
+ change.reviewers = groups_helper.pick_reviewer(group_data, change.identifiers) if group_data
end
yield(change)
end
end
- def before_cuttoff?(milestone)
- Gem::Version.new(milestone) < Gem::Version.new(milestone_ago(current_milestone, CUTOFF_MILESTONE_OLD))
- end
-
def fetch_gitlab_com_state(feature_flag_name)
# TBD
end
@@ -127,26 +121,6 @@ def feature_flag_grep(feature_flag_name)
)
end
- def current_milestone
- milestone = File.read(File.expand_path('../VERSION', __dir__))
- milestone.gsub(/^(\d+\.\d+).*$/, '\1').chomp
- end
-
- def milestone_ago(milestone, num_milestones)
- major, minor = milestone.split(".").map(&:to_i)
-
- older_major =
- if minor >= num_milestones
- major
- else
- major - (((num_milestones - minor - 1) / 13) + 1)
- end
-
- older_minor = (0..12).to_a[(minor - num_milestones) % 13]
-
- [older_major, older_minor].join(".")
- end
-
def assignees(rollout_issue_url)
rollout_issue = get_rollout_issue(rollout_issue_url)
@@ -184,6 +158,10 @@ def all_feature_flag_files
def groups_helper
@groups_helper ||= ::Keeps::Helpers::Groups.new
end
+
+ def milestones_helper
+ @milestones_helper ||= ::Keeps::Helpers::Milestones.new
+ end
end
end
# rubocop:enable Gitlab/Json
diff --git a/keeps/helpers/milestones.rb b/keeps/helpers/milestones.rb
new file mode 100644
index 0000000000000000000000000000000000000000..a6e4397b3c871e8fd8613caf29bcfd1fa7f58768
--- /dev/null
+++ b/keeps/helpers/milestones.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module Keeps
+ module Helpers
+ class Milestones
+ Error = Class.new(StandardError)
+
+ def before_cuttoff?(milestone:, milestones_ago:)
+ Gem::Version.new(milestone) < Gem::Version.new(past_milestone(milestones_ago: milestones_ago))
+ end
+
+ private
+
+ def current_milestone
+ milestone = File.read(File.expand_path('../../VERSION', __dir__))
+ milestone.gsub(/^(\d+\.\d+).*$/, '\1').chomp
+ end
+
+ def past_milestone(milestones_ago:)
+ major, minor = current_milestone.split(".").map(&:to_i)
+
+ older_major =
+ if minor >= milestones_ago
+ major
+ else
+ major - (((milestones_ago - minor - 1) / 13) + 1)
+ end
+
+ older_minor = (0..12).to_a[(minor - milestones_ago) % 13]
+
+ [older_major, older_minor].join(".")
+ end
+ end
+ end
+end
diff --git a/spec/keeps/helpers/milestones_spec.rb b/spec/keeps/helpers/milestones_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8ee2e29769ae9a4297db45b3888f699aa7ecd2cd
--- /dev/null
+++ b/spec/keeps/helpers/milestones_spec.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require './keeps/helpers/milestones'
+
+RSpec.describe Keeps::Helpers::Milestones, feature_category: :tooling do
+ before do
+ allow(File).to receive(:read).and_call_original
+ allow(File).to receive(:read).with(File.expand_path('../../../VERSION', __dir__)).and_return('16.9.0-pre')
+ end
+
+ describe '#before_cuttoff?' do
+ let(:milestone) { '16.9' }
+
+ subject(:before_cuttoff) { described_class.new.before_cuttoff?(milestone: milestone, milestones_ago: 12) }
+
+ it { is_expected.to eq(false) }
+
+ context 'when milestone is before cuttoff' do
+ let(:milestone) { '15.9' }
+
+ it { is_expected.to eq(true) }
+ end
+
+ context 'when milestone is more than 2 major versions before cuttoff' do
+ let(:milestone) { '14.12' }
+
+ it { is_expected.to eq(true) }
+ end
+ end
+end