Merge branches inside one repository using rugged instead of satellites
Signed-off-by: Dmitriy Zaporozhets dmitriy.zaporozhets@gmail.com
Merge request reports
Activity
Filter activity
mentioned in commit 804168e1
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 ofcan_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
mentioned in merge request !919 (merged)
mentioned in commit 459e6d34
Please register or sign in to reply