Skip to content

Remove cache param from MergeToRefService

Hordur Freyr Yngvason requested to merge remove-cache-param-from-merge-to-ref-service into master

What does this MR do and why?

This feature is never used. Past attempts at enabling it have been reverted. The first one caused an incident with merge trains, and the second caused a bug with stale merge request diffs.

The implementation was also incorrect, because the cache key did not contain all the parameters sent to the gitaly RPC, and some of these can vary between calls (for example, the user).

Since the code is both unused and incorrect, and the caching was not important enough to revisit this a third time over the last two years, we are removing it entirely.

Extra context

My personal motivation for the removal is that we are implementing a similar service that would also handle squashing and rebasing for merge train refs. This service would naturally be compared to MergeRequests::MergeToRefService, and having the unused caching out of the way simplifies things.

How to set up and validate locally

Check out this branch in your GDK.

  1. Enable merge trains in a project
  2. Create a merge request in the project
  3. Merge the merge request

If you managed to merge the merge request, then you have exercised both call sites to MergeRequests::MergeToRefService [1], [2].

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Hordur Freyr Yngvason

Merge request reports