Introduce a Keeps::Helpers::Milestones class

Signed-off-by: Rémy Coutable <remy@rymai.me>

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

keeps/helpers/milestones.rb

+# 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

spec/keeps/helpers/milestones_spec.rb

+# 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