Commit 43b75b38 authored by Rémy Coutable's avatar Rémy Coutable
Browse files

Don't even check if the branch exists locally as we only use its name in GH import 



The benefit is that we don't even have to create temp source/target
branches.

Also, when the source branch of the imported MR is from a fork, name it
"user:branch" to mimic how we display it for MR when forks actually
exists.

Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent f8184cec

lib/github/import.rb

+0 −4
Original line number Diff line number Diff line
@@ -144,8 +144,6 @@ def fetch_pull_requests 
          next unless merge_request.new_record? && pull_request.valid?



          begin

            pull_request.restore_branches!



            author_id   = user_id(pull_request.author, project.creator_id)

            description = format_description(pull_request.description, pull_request.author)
@@ -174,8 +172,6 @@ def fetch_pull_requests 
            fetch_comments(merge_request, :review_comment, review_comments_url, LegacyDiffNote)

          rescue => e

            error(:pull_request, pull_request.url, e.message)

          ensure

            pull_request.remove_tmp_branches!

          end

        end

lib/github/representation/pull_request.rb

+4 −52
Original line number Diff line number Diff line
@@ -9,13 +9,9 @@ def source_project 
      end



      def source_branch_name

        @source_branch_name ||= source_branch_exists? ? source_branch_ref : tmp_source_branch

      end



      def source_branch_exists?

        return @source_branch_exists if defined?(@source_branch_exists)



        @source_branch_exists = !cross_project? && source_branch.exists?

        # Mimic the "user:branch" displayed in the MR widget,

        # i.e. "Request to merge rymai:add-external-mounts into master"

        cross_project? ? "#{source_branch_user}:#{source_branch_ref}" : source_branch_ref

      end



      def target_project
@@ -23,13 +19,7 @@ def target_project 
      end



      def target_branch_name

        @target_branch_name ||= target_branch_exists? ? target_branch_ref : tmp_target_branch

      end



      def target_branch_exists?

        return @target_branch_exists if defined?(@target_branch_exists)



        @target_branch_exists ||= target_branch.exists?

        target_branch_ref

      end



      def state
@@ -47,18 +37,6 @@ def valid? 
        source_branch.valid? && target_branch.valid?

      end



      def restore_branches!

        restore_source_branch!

        restore_target_branch!

      end



      def remove_tmp_branches!

        return if opened?



        remove_tmp_source_branch!

        remove_tmp_target_branch!

      end



      private



      def project
@@ -78,32 +56,6 @@ def cross_project? 


        source_branch_repo.id != target_branch_repo.id

      end



      def restore_source_branch!

        return if source_branch_exists?



        source_branch.restore!(source_branch_name)

      end



      def restore_target_branch!

        return if target_branch_exists?



        target_branch.restore!(target_branch_name)

      end



      def remove_tmp_source_branch!

        # We should remove the source/target branches only if they were

        # restored. Otherwise, we'll remove branches like 'master' that

        # target_branch_exists? returns true. In other words, we need

        # to clean up only the restored branches that (source|target)_branch_exists?

        # returns false for the first time it has been called, because of

        # this that is important to memoize these values.

        source_branch.remove!(source_branch_name) unless source_branch_exists?

      end



      def remove_tmp_target_branch!

        target_branch.remove!(target_branch_name) unless target_branch_exists?

      end

    end

  end

end