[go: up one dir, main page]

Skip to content

Move MergeRequest#check_mergeability out of MergeRequest model

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

The following discussion from !21026 (merged) should be addressed:

  • @nick.thomas started a discussion: (+1 comment)

    When making the rebase asynchronous, I tried to move the enqueuing method out of the model and into a class method of the worker or service. I was defeated by the fact that I was using pessimistic locking and other database-level stuff though.

    Since we're doing none of that here, WDYT to reducing the size of app/models/merge_request.rb slightly by moving this to worker or service?

    It also means less code wrapped in a rubocop: disable CodeReuse/ServiceClass block.


This is to reduce the size of the MergeRequest model and move the responsibility out of the model as well.

Based on the discussion above, in order to move the MergeRequest#check_mergeability out of the model, we also need to take this into consideration:

This MergeRequest#check_mergeability method is also being called in MergeRequest#mergeable?. If we move this logic out, the #mergeable? method will then need to call that service class again which will require us to disable CodeReuse/ServiceClass cop. It seems more refactoring needs to be done aside from moving #check_mergeability.

Edited by 🤖 GitLab Bot 🤖