Skip to content
Snippets Groups Projects

Merge branches inside one repository using rugged instead of satellites

Merged username-removed-444 requested to merge use-rugged-for-merge into master

Signed-off-by: Dmitriy Zaporozhets dmitriy.zaporozhets@gmail.com

cc @rspeicher @DouweM

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
205 205 end
206 206
207 207 def check_if_can_be_merged
208 if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
208 can_be_merged =
209 if for_fork?
210 Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
211 else
212 rugged = project.repository.rugged
213 our_commit = rugged.branches[target_branch].target
214 their_commit = rugged.branches[source_branch].target
215
216 if our_commit && their_commit
217 !rugged.merge_commits(our_commit, their_commit).conflicts?
218 end
219 end
  • The if/else statement assigned to can_be_merged is too long and has multiple nested assignments of its own, which makes it not very clear that the value of can_be_merged will end up as !rugged.merge_commits(our_commit, their_commit).conflicts?

    More readable would be:

    if for_fork?
      can_be_merged = Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
    else
      rugged = project.repository.rugged
      our_commit = rugged.branches[target_branch].target
      their_commit = rugged.branches[source_branch].target
    
      if our_commit && their_commit
        can_be_merged = !rugged.merge_commits(our_commit, their_commit).conflicts?
      end
    end

    It would be even better if the in-repository steps would be moved to a method on Repository, since ideally we wouldn't deal with Rugged calls outside of Repository (Law of Demeter):

    class Repository
      def can_be_merged?(source_branch, target_branch = self.root_ref)
        our_commit = rugged.branches[target_branch].target
        their_commit = rugged.branches[source_branch].target
    
        return false unless our_commit && their_commit
    
        !rugged.merge_commits(our_commit, their_commit).conflicts?
      end
    end
    can_be_merged =
      if for_fork?
        Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
      else
        project.repository.can_be_merged?(source_branch, target)
      end
  • username-removed-444 mentioned in merge request !919 (merged)

    mentioned in merge request !919 (merged)

  • mentioned in commit 459e6d34

  • Please register or sign in to reply
    Loading