Refactor Ci::Build#shared_runners_minutes_limit_enabled?
The following discussion from !65155 (merged) should be addressed:
-
@fabiopitino started a discussion: This could be a follow-up: I'm wondering whether now makes more sense to move this method into
Ci::Minutes::CostFactor#for_project
where wereturn 0.0 unless project.shared_runners_minutes_limit_enabled?
.Then a cost factor of
0.0
is equivalent to not counting CI minutes consumption. This way the logic is in one place rather than remembering to useproject.shared_runners_minutes_limit_enabled?
as a guard clause.
Proposal
We could turn this method into:
class Ci::Build
def cost_factor_enabled?(project)
runner&.cost_factor_enabled?(project)
end
end
class Ci::Minutes::CostFactor
def for_project(project)
return 0.0 unless @runner_matcher.instance_type?
return 0.0 unless project.shared_runners_minutes_limit_enabled?
cost_factor = for_visibility(project.visibility_level)
if cost_factor == 0 && project.force_cost_factor?
NEW_NAMESPACE_PUBLIC_PROJECT_COST_FACTOR
else
cost_factor
end
end
end
And replace usage of build.shared_runners_minutes_limit_enabled?
with build.cost_factor_enabled?(project)
. This change makes sense because we are more interested in the cost factor for CI minutes consumption calculation rather than whether minutes limit is enabled.