Proposal: Rename Ci::Pipeline#merge_base_pipeline to #merge_target_pipeline

In %13.5 we introduced Ci::Pipeline#merge_base_pipeline for using the merge request's merge ref as a comparison base in merge request widgets. There was a discussion at the time of the name not being perfect, but we chose one for the sake of proving out the fix and deploying it for use with our CodeQuality merge request widget.

Now that it's been in place and successfully used, we should return to the issue of the confusing name.

At the time, merge_base_pipeline was used, because it's intent was to replace the base_pipeline when pipelines for merged results were being used in merge requests. But as this recent merge request makes more clear, the pipeline we're using is run on the merge target, i.e. the target_sha of the merge request pipeline:

  def merge_base_pipeline
    @merge_base_pipeline ||= project.ci_pipelines
      .order(id: :desc)
      .find_by(sha: actual_head_pipeline.target_sha, ref: target_branch)
  end

And we happen to be using this particular pipeline as the comparison base for the merge request:

  def comparison_base(service_class)
    return base_pipeline unless service_class == "Ci::CompareCodequalityReportsService"

    if actual_head_pipeline&.for_merged_result? && use_merge_base_pipeline?
      merge_base_pipeline
    else
      base_pipeline
    end
  end

So the name merge_base is really conflating two meanings:

  1. The pipeline itself points to a target commit in the merge request target branch
  2. The pipeline, in this context, is being used as a base for comparison.

As an example of increased legibility and self documentation after this change, the above method will (eventually) read:

class MergeRequest
  ...

  def comparison_base(service_class)
    if actual_head_pipeline&.for_merged_result?
      merge_target_pipeline
    else
      base_pipeline
    end
  end
Edited by drew stachon