From 1b4cd4c7d33469dd95f3360373f3374275e89a4a Mon Sep 17 00:00:00 2001
From: Peter Bittner <peter.bittner@vshn.ch>
Date: Thu, 6 Aug 2020 19:04:47 +0200
Subject: [PATCH 01/21] Tests: Move PR/MR related features to a dedicated file

---
 features/update.feature              | 22 ----------------------
 features/update/pull_request.feature | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 22 deletions(-)
 create mode 100644 features/update/pull_request.feature

diff --git a/features/update.feature b/features/update.feature
index 653eaf48..f264bee9 100644
--- a/features/update.feature
+++ b/features/update.feature
@@ -713,28 +713,6 @@ Feature: update
     And the puppet module "puppet-test" from "fakenamespace" should have only 1 commit made by "Aruba"
     And the puppet module "puppet-test" from "fakenamespace" should have 1 commit made by "Aruba" in branch "test"
 
-  Scenario: Creating a GitHub PR with an update
-    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
-    And a directory named "moduleroot"
-    And I set the environment variables to:
-      | variable     | value  |
-      | GITHUB_TOKEN | foobar |
-    When I run `msync update --noop --branch managed_update --pr`
-    Then the output should contain "Would submit PR "
-    And the exit status should be 0
-    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
-
-  Scenario: Creating a GitLab MR with an update
-    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
-    And a directory named "moduleroot"
-    And I set the environment variables to:
-      | variable     | value  |
-      | GITLAB_TOKEN | foobar |
-    When I run `msync update --noop --branch managed_update --pr`
-    Then the output should contain "Would submit MR "
-    And the exit status should be 0
-    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
-
   Scenario: Repository with a default branch other than master
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
     And the puppet module "puppet-test" from "fakenamespace" has the default branch named "develop"
diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
new file mode 100644
index 00000000..ff776f0f
--- /dev/null
+++ b/features/update/pull_request.feature
@@ -0,0 +1,23 @@
+Feature: Create a pull-request/merge-request after update
+
+  Scenario: Creating a GitHub PR with an update
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a directory named "moduleroot"
+    And I set the environment variables to:
+      | variable     | value  |
+      | GITHUB_TOKEN | foobar |
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the output should contain "Would submit PR "
+    And the exit status should be 0
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
+
+  Scenario: Creating a GitLab MR with an update
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a directory named "moduleroot"
+    And I set the environment variables to:
+      | variable     | value  |
+      | GITLAB_TOKEN | foobar |
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the output should contain "Would submit MR "
+    And the exit status should be 0
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"

From 7aa59da6c98aadbf711afb925b4297bf815b8b3a Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Sun, 3 Jan 2021 14:57:24 +0100
Subject: [PATCH 02/21] Tests: Add a PR/MR scenario when no credentials are
 given

---
 features/update/pull_request.feature | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index ff776f0f..2a83e0ab 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -21,3 +21,17 @@ Feature: Create a pull-request/merge-request after update
     Then the output should contain "Would submit MR "
     And the exit status should be 0
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
+
+  Scenario: Ask for PR without credentials
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        gitlab: {}
+      """
+    And a directory named "moduleroot"
+    When I run `msync update --noop --pr`
+    Then the stderr should contain "No GitLab token specified to create a merge request"
+    And the exit status should be 1
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"

From 1eb4c21215106f31fcea4d76aab0dccabea1929c Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Sun, 3 Jan 2021 14:59:22 +0100
Subject: [PATCH 03/21] Tests: Add a PR/MR scenario with modules from GitHub
 and GitLab

---
 features/update/pull_request.feature | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 2a83e0ab..ea7d73f6 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -35,3 +35,24 @@ Feature: Create a pull-request/merge-request after update
     Then the stderr should contain "No GitLab token specified to create a merge request"
     And the exit status should be 1
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
+
+  Scenario: Ask for PR/MR with modules from GitHub and from GitLab
+    Given a basic setup with a puppet module "puppet-github" from "fakenamespace"
+    And a basic setup with a puppet module "puppet-gitlab" from "fakenamespace"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-github:
+        github:
+          token: 'secret'
+      puppet-gitlab:
+        gitlab:
+          token: 'secret'
+      """
+    And a directory named "moduleroot"
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the exit status should be 0
+    And the output should contain "Would submit PR "
+    And the output should contain "Would submit MR "
+    And the puppet module "puppet-github" from "fakenamespace" should have no commits made by "Aruba"
+    And the puppet module "puppet-gitlab" from "fakenamespace" should have no commits made by "Aruba"

From af833b3db39aa5e75138f1f9501716530d70294a Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Thu, 31 Dec 2020 20:12:44 +0100
Subject: [PATCH 04/21] Refactor PR/MR related code (1/2)

This commit starts the refactoring of PR/MR feature code.

It provides a more readable code by:
- renaming method #manage for GitHub and GitLab class to open_pull_request
- extracting the credentials and endpoints to use in only one method: GitService#instantiate

It also provides a more understandable messages:
- `msync` does not warn anymore if no environmment variables are sets, but correctly configured in module options
- when no tokens are provided, the message now only contains the service that requires to specify token

Note: This message could be improved to help the user to setup its
modules configuration or ask him to set the environment variable.
---
 features/update/pull_request.feature     | 16 +++++++-
 lib/modulesync.rb                        | 51 +-----------------------
 lib/modulesync/git_service.rb            | 24 +++++++++++
 lib/modulesync/pr/github.rb              |  4 +-
 lib/modulesync/pr/gitlab.rb              |  4 +-
 lib/modulesync/source_code.rb            |  6 +++
 spec/unit/modulesync/git_service_spec.rb | 15 +++++++
 spec/unit/modulesync/pr/github_spec.rb   |  8 ++--
 spec/unit/modulesync/pr/gitlab_spec.rb   |  8 ++--
 spec/unit/modulesync_spec.rb             | 12 ------
 10 files changed, 75 insertions(+), 73 deletions(-)
 create mode 100644 lib/modulesync/git_service.rb
 create mode 100644 spec/unit/modulesync/git_service_spec.rb

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index ea7d73f6..329ae79e 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -1,7 +1,13 @@
 Feature: Create a pull-request/merge-request after update
 
-  Scenario: Creating a GitHub PR with an update
+  Scenario: Run update in no-op mode and ask for GitHub PR
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        github: {}
+      """
     And a directory named "moduleroot"
     And I set the environment variables to:
       | variable     | value  |
@@ -11,9 +17,15 @@ Feature: Create a pull-request/merge-request after update
     And the exit status should be 0
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 
-  Scenario: Creating a GitLab MR with an update
+  Scenario: Run update in no-op mode and ask for GitLab MR
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
     And a directory named "moduleroot"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        gitlab: {}
+      """
     And I set the environment variables to:
       | variable     | value  |
       | GITLAB_TOKEN | foobar |
diff --git a/lib/modulesync.rb b/lib/modulesync.rb
index 25baa1e4..31733ce7 100644
--- a/lib/modulesync.rb
+++ b/lib/modulesync.rb
@@ -130,8 +130,7 @@ def self.manage_module(puppet_module, module_files, defaults)
     if options[:noop]
       puts "Using no-op. Files in '#{puppet_module.given_name}' may be changed but will not be committed."
       puppet_module.repository.show_changes(options)
-      options[:pr] && \
-        pr(puppet_module).manage(puppet_module.repository_namespace, puppet_module.repository_name, options)
+      options[:pr] && puppet_module.open_pull_request
     elsif !options[:offline]
       pushed = puppet_module.repository.submit_changes(files_to_manage, options)
       # Only bump/tag if pushing didn't fail (i.e. there were changes)
@@ -139,8 +138,7 @@ def self.manage_module(puppet_module, module_files, defaults)
         new = puppet_module.bump(options[:message], options[:changelog])
         puppet_module.repository.tag(new, options[:tag_pattern]) if options[:tag]
       end
-      pushed && options[:pr] && \
-        pr(puppet_module).manage(puppet_module.repository_namespace, puppet_module.repository_name, options)
+      pushed && options[:pr] && puppet_module.open_pull_request
     end
   end
 
@@ -157,15 +155,6 @@ def self.update(cli_options)
     @options = config_defaults.merge(cli_options)
     defaults = Util.parse_config(config_path(CONF_FILE, options))
 
-    if options[:pr]
-      unless options[:branch]
-        $stderr.puts 'A branch must be specified with --branch to use --pr!'
-        raise
-      end
-
-      @pr = create_pr_manager if options[:pr]
-    end
-
     local_template_dir = config_path(MODULE_FILES_DIR, options)
     local_files = find_template_files(local_template_dir)
     module_files = relative_names(local_files, local_template_dir)
@@ -189,40 +178,4 @@ def self.update(cli_options)
     end
     exit 1 if errors && options[:fail_on_warnings]
   end
-
-  def self.pr(puppet_module)
-    module_options = puppet_module.options
-    github_conf = module_options[:github]
-    gitlab_conf = module_options[:gitlab]
-
-    if !github_conf.nil?
-      base_url = github_conf[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
-      require 'modulesync/pr/github'
-      ModuleSync::PR::GitHub.new(github_conf[:token], base_url)
-    elsif !gitlab_conf.nil?
-      base_url = gitlab_conf[:base_url] || ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')
-      require 'modulesync/pr/gitlab'
-      ModuleSync::PR::GitLab.new(gitlab_conf[:token], base_url)
-    elsif @pr.nil?
-      $stderr.puts 'No GitHub or GitLab token specified for --pr!'
-      raise
-    else
-      @pr
-    end
-  end
-
-  def self.create_pr_manager
-    github_token = ENV.fetch('GITHUB_TOKEN', '')
-    gitlab_token = ENV.fetch('GITLAB_TOKEN', '')
-
-    if !github_token.empty?
-      require 'modulesync/pr/github'
-      ModuleSync::PR::GitHub.new(github_token, ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com'))
-    elsif !gitlab_token.empty?
-      require 'modulesync/pr/gitlab'
-      ModuleSync::PR::GitLab.new(gitlab_token, ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4'))
-    else
-      warn '--pr specified without environment variables GITHUB_TOKEN or GITLAB_TOKEN'
-    end
-  end
 end
diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
new file mode 100644
index 00000000..21911230
--- /dev/null
+++ b/lib/modulesync/git_service.rb
@@ -0,0 +1,24 @@
+module ModuleSync
+  # Namespace for Git service classes (ie. GitHub, GitLab)
+  module GitService
+    def self.instantiate(type:, options:)
+      options ||= {}
+      case type
+      when :github
+        endpoint = options[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
+        token = options[:token] || ENV['GITHUB_TOKEN']
+        raise ModuleSync::Error, 'No GitHub token specified to create a pull request' if token.nil?
+        require 'modulesync/pr/github'
+        return ModuleSync::PR::GitHub.new(token, endpoint)
+      when :gitlab
+        endpoint = options[:base_url] || ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')
+        token = options[:token] || ENV['GITLAB_TOKEN']
+        raise ModuleSync::Error, 'No GitLab token specified to create a merge request' if token.nil?
+        require 'modulesync/pr/gitlab'
+        return ModuleSync::PR::GitLab.new(token, endpoint)
+      else
+        raise ModuleSync::Error, "Unable to manage a PR/MR for Git service: '#{type}'"
+      end
+    end
+  end
+end
diff --git a/lib/modulesync/pr/github.rb b/lib/modulesync/pr/github.rb
index eaffd6c9..9726068f 100644
--- a/lib/modulesync/pr/github.rb
+++ b/lib/modulesync/pr/github.rb
@@ -1,3 +1,5 @@
+require 'modulesync/git_service'
+
 require 'octokit'
 require 'modulesync/util'
 
@@ -13,7 +15,7 @@ def initialize(token, endpoint)
         @api = Octokit::Client.new(:access_token => token)
       end
 
-      def manage(namespace, module_name, options)
+      def open_pull_request(namespace, module_name, options)
         repo_path = File.join(namespace, module_name)
         branch = options[:remote_branch] || options[:branch]
         head = "#{namespace}:#{branch}"
diff --git a/lib/modulesync/pr/gitlab.rb b/lib/modulesync/pr/gitlab.rb
index 571a6795..658cc75f 100644
--- a/lib/modulesync/pr/gitlab.rb
+++ b/lib/modulesync/pr/gitlab.rb
@@ -1,3 +1,5 @@
+require 'modulesync/git_service'
+
 require 'gitlab'
 require 'modulesync/util'
 
@@ -13,7 +15,7 @@ def initialize(token, endpoint)
         )
       end
 
-      def manage(namespace, module_name, options)
+      def open_pull_request(namespace, module_name, options)
         repo_path = File.join(namespace, module_name)
         branch = options[:remote_branch] || options[:branch]
         head = "#{namespace}:#{branch}"
diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb
index 7415d86b..ff6b0ccf 100644
--- a/lib/modulesync/source_code.rb
+++ b/lib/modulesync/source_code.rb
@@ -1,4 +1,5 @@
 require 'modulesync'
+require 'modulesync/git_service'
 require 'modulesync/repository'
 require 'modulesync/util'
 
@@ -47,6 +48,11 @@ def path(*parts)
       File.join(working_directory, *parts)
     end
 
+    def open_pull_request
+      git_service = GitService.instantiate type: git_service_type, options: @options[git_service_type]
+      git_service.open_pull_request(repository_namespace, repository_name, ModuleSync.options)
+    end
+
     private
 
     def _repository_remote
diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb
new file mode 100644
index 00000000..eee64366
--- /dev/null
+++ b/spec/unit/modulesync/git_service_spec.rb
@@ -0,0 +1,15 @@
+require 'modulesync/git_service'
+
+describe ModuleSync::GitService do
+  context 'when instantiate a GitHub service without credentials' do
+    it 'raises an error' do
+      expect { ModuleSync::GitService.instantiate(type: :github, options: nil) }.to raise_error(ModuleSync::Error, 'No GitHub token specified to create a pull request')
+    end
+  end
+
+  context 'when instantiate a GitLab service without credentials' do
+    it 'raises an error' do
+      expect { ModuleSync::GitService.instantiate(type: :gitlab, options: nil) }.to raise_error(ModuleSync::Error, 'No GitLab token specified to create a merge request')
+    end
+  end
+end
diff --git a/spec/unit/modulesync/pr/github_spec.rb b/spec/unit/modulesync/pr/github_spec.rb
index 830c4b05..5660f86e 100644
--- a/spec/unit/modulesync/pr/github_spec.rb
+++ b/spec/unit/modulesync/pr/github_spec.rb
@@ -2,7 +2,7 @@
 require 'modulesync/pr/github'
 
 describe ModuleSync::PR::GitHub do
-  context '::manage' do
+  context '::open_pull_request' do
     before(:each) do
       @git_repo = 'test/modulesync'
       @namespace, @repo_name = @git_repo.split('/')
@@ -22,7 +22,7 @@
     it 'submits PR when --pr is set' do
       allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
       expect(@client).to receive(:create_pull_request).with(@git_repo, 'master', @options[:branch], @options[:pr_title], @options[:message]).and_return({"html_url" => "http://example.com/pulls/22"})
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout
     end
 
     it 'skips submitting PR if one has already been issued' do
@@ -33,7 +33,7 @@
       }
 
       expect(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([pr])
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
     end
 
     it 'adds labels to PR when --pr-labels is set' do
@@ -43,7 +43,7 @@
       allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
 
       expect(@client).to receive(:add_labels_to_an_issue).with(@git_repo, "44", ["HELLO", "WORLD"])
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
     end
   end
 end
diff --git a/spec/unit/modulesync/pr/gitlab_spec.rb b/spec/unit/modulesync/pr/gitlab_spec.rb
index 360a773b..53420b0a 100644
--- a/spec/unit/modulesync/pr/gitlab_spec.rb
+++ b/spec/unit/modulesync/pr/gitlab_spec.rb
@@ -2,7 +2,7 @@
 require 'modulesync/pr/gitlab'
 
 describe ModuleSync::PR::GitLab do
-  context '::manage' do
+  context '::open_pull_request' do
     before(:each) do
       @git_repo = 'test/modulesync'
       @namespace, @repo_name = @git_repo.split('/')
@@ -35,7 +35,7 @@
               :target_branch => 'master',
              ).and_return({"html_url" => "http://example.com/pulls/22"})
 
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Submitted MR/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Submitted MR/).to_stdout
     end
 
     it 'skips submitting MR if one has already been issued' do
@@ -52,7 +52,7 @@
               :target_branch => 'master',
              ).and_return([mr])
 
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout
     end
 
     it 'adds labels to MR when --pr-labels is set' do
@@ -75,7 +75,7 @@
               :target_branch => 'master',
              ).and_return([])
 
-      expect { @it.manage(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
+      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
     end
   end
 end
diff --git a/spec/unit/modulesync_spec.rb b/spec/unit/modulesync_spec.rb
index 42b82168..34a559e5 100644
--- a/spec/unit/modulesync_spec.rb
+++ b/spec/unit/modulesync_spec.rb
@@ -11,16 +11,4 @@
       ModuleSync.update(options)
     end
   end
-
-  context '::pr' do
-    describe "Raise Error" do
-      let(:puppet_module) do
-        ModuleSync::PuppetModule.new 'puppet-test', remote: 'dummy'
-      end
-
-      it 'raises an error when neither GITHUB_TOKEN nor GITLAB_TOKEN are set for PRs' do
-        expect { ModuleSync.pr(puppet_module) }.to raise_error(RuntimeError).and output(/No GitHub or GitLab token specified for --pr/).to_stderr
-      end
-    end
-  end
 end

From ed8e4d215429e755af8164f3f936e8c4d829606c Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 12:20:15 +0100
Subject: [PATCH 05/21] Refactor PR/MR related code (2/2)

This commit renames ModuleSync::PR module to ModuleSync::GitService.

Git service classes (ie. GitHub and GitLab) can now be used to implement
more features than PR/MR opening. (e.g #158)
---
 lib/modulesync/git_service.rb                           | 8 ++++----
 lib/modulesync/{pr => git_service}/github.rb            | 2 +-
 lib/modulesync/{pr => git_service}/gitlab.rb            | 2 +-
 spec/unit/modulesync/{pr => git_service}/github_spec.rb | 7 ++++---
 spec/unit/modulesync/{pr => git_service}/gitlab_spec.rb | 9 +++++----
 5 files changed, 15 insertions(+), 13 deletions(-)
 rename lib/modulesync/{pr => git_service}/github.rb (99%)
 rename lib/modulesync/{pr => git_service}/gitlab.rb (99%)
 rename spec/unit/modulesync/{pr => git_service}/github_spec.rb (92%)
 rename spec/unit/modulesync/{pr => git_service}/gitlab_spec.rb (92%)

diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
index 21911230..1a512939 100644
--- a/lib/modulesync/git_service.rb
+++ b/lib/modulesync/git_service.rb
@@ -8,14 +8,14 @@ def self.instantiate(type:, options:)
         endpoint = options[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
         token = options[:token] || ENV['GITHUB_TOKEN']
         raise ModuleSync::Error, 'No GitHub token specified to create a pull request' if token.nil?
-        require 'modulesync/pr/github'
-        return ModuleSync::PR::GitHub.new(token, endpoint)
+        require 'modulesync/git_service/github'
+        ModuleSync::GitService::GitHub.new(token, endpoint)
       when :gitlab
         endpoint = options[:base_url] || ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')
         token = options[:token] || ENV['GITLAB_TOKEN']
         raise ModuleSync::Error, 'No GitLab token specified to create a merge request' if token.nil?
-        require 'modulesync/pr/gitlab'
-        return ModuleSync::PR::GitLab.new(token, endpoint)
+        require 'modulesync/git_service/gitlab'
+        ModuleSync::GitService::GitLab.new(token, endpoint)
       else
         raise ModuleSync::Error, "Unable to manage a PR/MR for Git service: '#{type}'"
       end
diff --git a/lib/modulesync/pr/github.rb b/lib/modulesync/git_service/github.rb
similarity index 99%
rename from lib/modulesync/pr/github.rb
rename to lib/modulesync/git_service/github.rb
index 9726068f..dfa1105e 100644
--- a/lib/modulesync/pr/github.rb
+++ b/lib/modulesync/git_service/github.rb
@@ -4,7 +4,7 @@
 require 'modulesync/util'
 
 module ModuleSync
-  module PR
+  module GitService
     # GitHub creates and manages pull requests on github.com or GitHub
     # Enterprise installations.
     class GitHub
diff --git a/lib/modulesync/pr/gitlab.rb b/lib/modulesync/git_service/gitlab.rb
similarity index 99%
rename from lib/modulesync/pr/gitlab.rb
rename to lib/modulesync/git_service/gitlab.rb
index 658cc75f..6ada5559 100644
--- a/lib/modulesync/pr/gitlab.rb
+++ b/lib/modulesync/git_service/gitlab.rb
@@ -4,7 +4,7 @@
 require 'modulesync/util'
 
 module ModuleSync
-  module PR
+  module GitService
     # GitLab creates and manages merge requests on gitlab.com or private GitLab
     # installations.
     class GitLab
diff --git a/spec/unit/modulesync/pr/github_spec.rb b/spec/unit/modulesync/git_service/github_spec.rb
similarity index 92%
rename from spec/unit/modulesync/pr/github_spec.rb
rename to spec/unit/modulesync/git_service/github_spec.rb
index 5660f86e..f66c574b 100644
--- a/spec/unit/modulesync/pr/github_spec.rb
+++ b/spec/unit/modulesync/git_service/github_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
-require 'modulesync/pr/github'
 
-describe ModuleSync::PR::GitHub do
+require 'modulesync/git_service/github'
+
+describe ModuleSync::GitService::GitHub do
   context '::open_pull_request' do
     before(:each) do
       @git_repo = 'test/modulesync'
@@ -16,7 +17,7 @@
 
       @client = double()
       allow(Octokit::Client).to receive(:new).and_return(@client)
-      @it = ModuleSync::PR::GitHub.new('test', 'https://api.github.com')
+      @it = ModuleSync::GitService::GitHub.new('test', 'https://api.github.com')
     end
 
     it 'submits PR when --pr is set' do
diff --git a/spec/unit/modulesync/pr/gitlab_spec.rb b/spec/unit/modulesync/git_service/gitlab_spec.rb
similarity index 92%
rename from spec/unit/modulesync/pr/gitlab_spec.rb
rename to spec/unit/modulesync/git_service/gitlab_spec.rb
index 53420b0a..29008f6b 100644
--- a/spec/unit/modulesync/pr/gitlab_spec.rb
+++ b/spec/unit/modulesync/git_service/gitlab_spec.rb
@@ -1,14 +1,15 @@
 require 'spec_helper'
-require 'modulesync/pr/gitlab'
 
-describe ModuleSync::PR::GitLab do
+require 'modulesync/git_service/gitlab'
+
+describe ModuleSync::GitService::GitLab do
   context '::open_pull_request' do
     before(:each) do
       @git_repo = 'test/modulesync'
       @namespace, @repo_name = @git_repo.split('/')
       @options = {
         :pr => true,
-        :pr_title => 'Test PR is submitted',
+        :pr_title => 'Test MR is submitted',
         :branch => 'test',
         :message => 'Hello world',
         :pr_auto_merge => false,
@@ -16,7 +17,7 @@
 
       @client = double()
       allow(Gitlab::Client).to receive(:new).and_return(@client)
-      @it = ModuleSync::PR::GitLab.new('test', 'https://gitlab.com/api/v4')
+      @it = ModuleSync::GitService::GitLab.new('test', 'https://gitlab.com/api/v4')
     end
 
     it 'submits MR when --pr is set' do

From 1173a47892327b7a83e66d7d8f4efd64422aa842 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 14:47:09 +0100
Subject: [PATCH 06/21] Spec: Use multiline notation in GitHub specs

This commit only changes notation to ease code review, it does not
change any code.
---
 .../modulesync/git_service/github_spec.rb     | 35 ++++++++++++++++---
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/spec/unit/modulesync/git_service/github_spec.rb b/spec/unit/modulesync/git_service/github_spec.rb
index f66c574b..41f14dca 100644
--- a/spec/unit/modulesync/git_service/github_spec.rb
+++ b/spec/unit/modulesync/git_service/github_spec.rb
@@ -21,8 +21,19 @@
     end
 
     it 'submits PR when --pr is set' do
-      allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
-      expect(@client).to receive(:create_pull_request).with(@git_repo, 'master', @options[:branch], @options[:pr_title], @options[:message]).and_return({"html_url" => "http://example.com/pulls/22"})
+      allow(@client).to receive(:pull_requests)
+        .with(@git_repo,
+              :state => 'open',
+              :base => 'master',
+              :head => "#{@namespace}:#{@options[:branch]}"
+             ).and_return([])
+      expect(@client).to receive(:create_pull_request)
+        .with(@git_repo,
+              'master',
+              @options[:branch],
+              @options[:pr_title],
+              @options[:message]
+             ).and_return({"html_url" => "http://example.com/pulls/22"})
       expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout
     end
 
@@ -33,7 +44,12 @@
         "number" => "44"
       }
 
-      expect(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([pr])
+      expect(@client).to receive(:pull_requests)
+        .with(@git_repo,
+              :state => 'open',
+              :base => 'master',
+              :head => "#{@namespace}:#{@options[:branch]}"
+             ).and_return([pr])
       expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
     end
 
@@ -41,9 +57,18 @@
       @options[:pr_labels] = "HELLO,WORLD"
 
       allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"})
-      allow(@client).to receive(:pull_requests).with(@git_repo, :state => 'open', :base => 'master', :head => "#{@namespace}:#{@options[:branch]}").and_return([])
+      allow(@client).to receive(:pull_requests)
+        .with(@git_repo,
+              :state => 'open',
+              :base => 'master',
+              :head => "#{@namespace}:#{@options[:branch]}"
+             ).and_return([])
 
-      expect(@client).to receive(:add_labels_to_an_issue).with(@git_repo, "44", ["HELLO", "WORLD"])
+      expect(@client).to receive(:add_labels_to_an_issue)
+        .with(@git_repo,
+              "44",
+              ["HELLO", "WORLD"]
+             )
       expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
     end
   end

From 29f5841f3385a4e93119952f7520d137656f16f5 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 14:50:49 +0100
Subject: [PATCH 07/21] Spec: Use rspec context in GitHub and GitLab specs

This commit only changes notation to ease further context introduction, it does not
change any code.
---
 .../modulesync/git_service/github_spec.rb     | 32 ++++++++-------
 .../modulesync/git_service/gitlab_spec.rb     | 39 ++++++++++---------
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/spec/unit/modulesync/git_service/github_spec.rb b/spec/unit/modulesync/git_service/github_spec.rb
index 41f14dca..0f809432 100644
--- a/spec/unit/modulesync/git_service/github_spec.rb
+++ b/spec/unit/modulesync/git_service/github_spec.rb
@@ -53,23 +53,25 @@
       expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
     end
 
-    it 'adds labels to PR when --pr-labels is set' do
-      @options[:pr_labels] = "HELLO,WORLD"
+    context 'when labels are set' do
+      it 'adds labels to PR' do
+        @options[:pr_labels] = "HELLO,WORLD"
 
-      allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"})
-      allow(@client).to receive(:pull_requests)
-        .with(@git_repo,
-              :state => 'open',
-              :base => 'master',
-              :head => "#{@namespace}:#{@options[:branch]}"
-             ).and_return([])
+        allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"})
+        allow(@client).to receive(:pull_requests)
+          .with(@git_repo,
+                :state => 'open',
+                :base => 'master',
+                :head => "#{@namespace}:#{@options[:branch]}"
+               ).and_return([])
 
-      expect(@client).to receive(:add_labels_to_an_issue)
-        .with(@git_repo,
-              "44",
-              ["HELLO", "WORLD"]
-             )
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
+        expect(@client).to receive(:add_labels_to_an_issue)
+          .with(@git_repo,
+                "44",
+                ["HELLO", "WORLD"]
+               )
+        expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
+      end
     end
   end
 end
diff --git a/spec/unit/modulesync/git_service/gitlab_spec.rb b/spec/unit/modulesync/git_service/gitlab_spec.rb
index 29008f6b..b8f05746 100644
--- a/spec/unit/modulesync/git_service/gitlab_spec.rb
+++ b/spec/unit/modulesync/git_service/gitlab_spec.rb
@@ -56,27 +56,30 @@
       expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout
     end
 
-    it 'adds labels to MR when --pr-labels is set' do
-      @options[:pr_labels] = "HELLO,WORLD"
-      mr = double()
-      allow(mr).to receive(:iid).and_return("42")
 
-      expect(@client).to receive(:create_merge_request)
-        .with(@git_repo,
-              @options[:pr_title],
-              :labels => ["HELLO", "WORLD"],
-              :source_branch => @options[:branch],
-              :target_branch => 'master',
-             ).and_return(mr)
+    context 'when labels are set' do
+      it 'adds labels to MR' do
+        @options[:pr_labels] = "HELLO,WORLD"
+        mr = double()
+        allow(mr).to receive(:iid).and_return("42")
 
-      allow(@client).to receive(:merge_requests)
-        .with(@git_repo,
-              :state => 'opened',
-              :source_branch => "#{@namespace}:#{@options[:branch]}",
-              :target_branch => 'master',
-             ).and_return([])
+        expect(@client).to receive(:create_merge_request)
+          .with(@git_repo,
+                @options[:pr_title],
+                :labels => ["HELLO", "WORLD"],
+                :source_branch => @options[:branch],
+                :target_branch => 'master',
+               ).and_return(mr)
+
+        allow(@client).to receive(:merge_requests)
+          .with(@git_repo,
+                :state => 'opened',
+                :source_branch => "#{@namespace}:#{@options[:branch]}",
+                :target_branch => 'master',
+               ).and_return([])
 
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
+        expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
+      end
     end
   end
 end

From 3fecd3e64d2bd762e8313c0ff26adab19f3585c1 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Sun, 3 Jan 2021 14:58:41 +0100
Subject: [PATCH 08/21] Tests: Add PR scenarios with same target and source
 branches

---
 features/update/pull_request.feature | 31 ++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 329ae79e..98f82e1b 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -68,3 +68,34 @@ Feature: Create a pull-request/merge-request after update
     And the output should contain "Would submit MR "
     And the puppet module "puppet-github" from "fakenamespace" should have no commits made by "Aruba"
     And the puppet module "puppet-gitlab" from "fakenamespace" should have no commits made by "Aruba"
+
+  Scenario: Ask for PR with same source and target branch
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        gitlab:
+          token: 'secret'
+      """
+    And a directory named "moduleroot"
+    When I run `msync update --noop --branch managed_update --pr --pr-target-branch managed_update`
+    Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'managed_update'"
+    And the exit status should be 1
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
+
+  Scenario: Ask for PR with the default branch as source and target
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And the puppet module "puppet-test" from "fakenamespace" has the default branch named "custom_default_branch"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        github:
+          token: 'secret'
+      """
+    And a directory named "moduleroot"
+    When I run `msync update --noop --pr`
+    Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'custom_default_branch'"
+    And the exit status should be 1
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"

From 61990dba98216a26adab19acf98c4a3c16a9a57e Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 13:53:02 +0100
Subject: [PATCH 09/21] Rework PR/MR related code (1/2)

This commit adds a check to ensure no PR/MR are made with the same source
and target branch.

This commit implements a facade class (ie. GitService::Base) to prevent
adding duplicate code in the git services and remove any global options[] usage.

It lets SourceCode#open_pull_request to craft the right parameters to
pass to the GitService once, so it deduplicates the code in specialised
service.

By design, this allows to honor easily the default repo branch.
---
 lib/modulesync/git_service/base.rb            | 24 ++++++++
 lib/modulesync/git_service/github.rb          | 37 ++++++------
 lib/modulesync/git_service/gitlab.rb          | 35 ++++++-----
 lib/modulesync/source_code.rb                 | 11 +++-
 .../modulesync/git_service/github_spec.rb     | 58 +++++++++---------
 .../modulesync/git_service/gitlab_spec.rb     | 59 ++++++++++---------
 6 files changed, 132 insertions(+), 92 deletions(-)
 create mode 100644 lib/modulesync/git_service/base.rb

diff --git a/lib/modulesync/git_service/base.rb b/lib/modulesync/git_service/base.rb
new file mode 100644
index 00000000..81ef8109
--- /dev/null
+++ b/lib/modulesync/git_service/base.rb
@@ -0,0 +1,24 @@
+module ModuleSync
+  module GitService
+    # Generic class for git services
+    class Base
+      def open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength
+        unless source_branch != target_branch
+          raise ModuleSync::Error,
+                "Unable to open a pull request with the same source and target branch: '#{source_branch}'"
+        end
+
+        _open_pull_request(
+          repo_path: repo_path,
+          namespace: namespace,
+          title: title,
+          message: message,
+          source_branch: source_branch,
+          target_branch: target_branch,
+          labels: labels,
+          noop: noop,
+        )
+      end
+    end
+  end
+end
diff --git a/lib/modulesync/git_service/github.rb b/lib/modulesync/git_service/github.rb
index dfa1105e..a98c7848 100644
--- a/lib/modulesync/git_service/github.rb
+++ b/lib/modulesync/git_service/github.rb
@@ -1,4 +1,5 @@
 require 'modulesync/git_service'
+require 'modulesync/git_service/base'
 
 require 'octokit'
 require 'modulesync/util'
@@ -7,7 +8,7 @@ module ModuleSync
   module GitService
     # GitHub creates and manages pull requests on github.com or GitHub
     # Enterprise installations.
-    class GitHub
+    class GitHub < Base
       def initialize(token, endpoint)
         Octokit.configure do |c|
           c.api_endpoint = endpoint
@@ -15,16 +16,15 @@ def initialize(token, endpoint)
         @api = Octokit::Client.new(:access_token => token)
       end
 
-      def open_pull_request(namespace, module_name, options)
-        repo_path = File.join(namespace, module_name)
-        branch = options[:remote_branch] || options[:branch]
-        head = "#{namespace}:#{branch}"
-        target_branch = options[:pr_target_branch] || 'master'
+      private
 
-        if options[:noop]
+      def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength
+        head = "#{namespace}:#{source_branch}"
+
+        if noop
           $stdout.puts \
-            "Using no-op. Would submit PR '#{options[:pr_title]}' to #{repo_path} " \
-            "- merges #{branch} into #{target_branch}"
+            "Using no-op. Would submit PR '#{title}' to '#{repo_path}' " \
+            "- merges '#{source_branch}' into '#{target_branch}'"
           return
         end
 
@@ -34,25 +34,24 @@ def open_pull_request(namespace, module_name, options)
                                            :head => head)
         unless pull_requests.empty?
           # Skip creating the PR if it exists already.
-          $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{branch}"
+          $stdout.puts "Skipped! #{pull_requests.length} PRs found for branch '#{source_branch}'"
           return
         end
 
-        pr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
         pr = @api.create_pull_request(repo_path,
                                       target_branch,
-                                      branch,
-                                      options[:pr_title],
-                                      options[:message])
+                                      source_branch,
+                                      title,
+                                      message)
         $stdout.puts \
-          "Submitted PR '#{options[:pr_title]}' to #{repo_path} " \
-          "- merges #{branch} into #{target_branch}"
+          "Submitted PR '#{title}' to '#{repo_path}' " \
+          "- merges #{source_branch} into #{target_branch}"
 
         # We only assign labels to the PR if we've discovered a list > 1. The labels MUST
         # already exist. We DO NOT create missing labels.
-        return if pr_labels.empty?
-        $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{pr_labels.join(', ')}"
-        @api.add_labels_to_an_issue(repo_path, pr['number'], pr_labels)
+        return if labels.empty?
+        $stdout.puts "Attaching the following labels to PR #{pr['number']}: #{labels.join(', ')}"
+        @api.add_labels_to_an_issue(repo_path, pr['number'], labels)
       end
     end
   end
diff --git a/lib/modulesync/git_service/gitlab.rb b/lib/modulesync/git_service/gitlab.rb
index 6ada5559..084ba99c 100644
--- a/lib/modulesync/git_service/gitlab.rb
+++ b/lib/modulesync/git_service/gitlab.rb
@@ -1,4 +1,5 @@
 require 'modulesync/git_service'
+require 'modulesync/git_service/base'
 
 require 'gitlab'
 require 'modulesync/util'
@@ -7,7 +8,7 @@ module ModuleSync
   module GitService
     # GitLab creates and manages merge requests on gitlab.com or private GitLab
     # installations.
-    class GitLab
+    class GitLab < Base
       def initialize(token, endpoint)
         @api = Gitlab::Client.new(
           :endpoint => endpoint,
@@ -15,16 +16,15 @@ def initialize(token, endpoint)
         )
       end
 
-      def open_pull_request(namespace, module_name, options)
-        repo_path = File.join(namespace, module_name)
-        branch = options[:remote_branch] || options[:branch]
-        head = "#{namespace}:#{branch}"
-        target_branch = options[:pr_target_branch] || 'master'
+      private
 
-        if options[:noop]
+      def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Lint/UnusedMethodArgument, Metrics/LineLength
+        head = "#{namespace}:#{source_branch}"
+
+        if noop
           $stdout.puts \
-            "Using no-op. Would submit MR '#{options[:pr_title]}' to #{repo_path} " \
-            "- merges #{branch} into #{target_branch}"
+            "Using no-op. Would submit MR '#{title}' to '#{repo_path}' " \
+            "- merges #{source_branch} into #{target_branch}"
           return
         end
 
@@ -34,22 +34,21 @@ def open_pull_request(namespace, module_name, options)
                                              :target_branch => target_branch)
         unless merge_requests.empty?
           # Skip creating the MR if it exists already.
-          $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{branch}"
+          $stdout.puts "Skipped! #{merge_requests.length} MRs found for branch '#{source_branch}'"
           return
         end
 
-        mr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
         mr = @api.create_merge_request(repo_path,
-                                       options[:pr_title],
-                                       :source_branch => branch,
+                                       title,
+                                       :source_branch => source_branch,
                                        :target_branch => target_branch,
-                                       :labels => mr_labels)
+                                       :labels => labels)
         $stdout.puts \
-          "Submitted MR '#{options[:pr_title]}' to #{repo_path} " \
-          "- merges #{branch} into #{target_branch}"
+          "Submitted MR '#{title}' to '#{repo_path}' " \
+          "- merges '#{source_branch}' into '#{target_branch}'"
 
-        return if mr_labels.empty?
-        $stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
+        return if labels.empty?
+        $stdout.puts "Attached the following labels to MR #{mr.iid}: #{labels.join(', ')}"
       end
     end
   end
diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb
index ff6b0ccf..ff487c5d 100644
--- a/lib/modulesync/source_code.rb
+++ b/lib/modulesync/source_code.rb
@@ -50,7 +50,16 @@ def path(*parts)
 
     def open_pull_request
       git_service = GitService.instantiate type: git_service_type, options: @options[git_service_type]
-      git_service.open_pull_request(repository_namespace, repository_name, ModuleSync.options)
+      git_service.open_pull_request(
+        repo_path: repository_path,
+        namespace: repository_namespace,
+        title: ModuleSync.options[:pr_title],
+        message: ModuleSync.options[:message],
+        source_branch: ModuleSync.options[:remote_branch] || ModuleSync.options[:branch] || repository.default_branch,
+        target_branch: ModuleSync.options[:pr_target_branch] || repository.default_branch,
+        labels: ModuleSync::Util.parse_list(ModuleSync.options[:pr_labels]),
+        noop: ModuleSync.options[:noop],
+      )
     end
 
     private
diff --git a/spec/unit/modulesync/git_service/github_spec.rb b/spec/unit/modulesync/git_service/github_spec.rb
index 0f809432..603ece11 100644
--- a/spec/unit/modulesync/git_service/github_spec.rb
+++ b/spec/unit/modulesync/git_service/github_spec.rb
@@ -5,36 +5,41 @@
 describe ModuleSync::GitService::GitHub do
   context '::open_pull_request' do
     before(:each) do
-      @git_repo = 'test/modulesync'
-      @namespace, @repo_name = @git_repo.split('/')
-      @options = {
-        :pr => true,
-        :pr_title => 'Test PR is submitted',
-        :branch => 'test',
-        :message => 'Hello world',
-        :pr_auto_merge => false,
-      }
-
       @client = double()
       allow(Octokit::Client).to receive(:new).and_return(@client)
       @it = ModuleSync::GitService::GitHub.new('test', 'https://api.github.com')
     end
 
+    let(:args) do
+      {
+        repo_path: 'test/modulesync',
+        namespace: 'test',
+        title: 'Test PR is submitted',
+        message: 'Hello world',
+        source_branch: 'test',
+        target_branch: 'master',
+        labels: labels,
+        noop: false,
+      }
+    end
+
+    let(:labels) { [] }
+
     it 'submits PR when --pr is set' do
       allow(@client).to receive(:pull_requests)
-        .with(@git_repo,
+        .with(args[:repo_path],
               :state => 'open',
               :base => 'master',
-              :head => "#{@namespace}:#{@options[:branch]}"
+              :head => "#{args[:namespace]}:#{args[:source_branch]}"
              ).and_return([])
       expect(@client).to receive(:create_pull_request)
-        .with(@git_repo,
+        .with(args[:repo_path],
               'master',
-              @options[:branch],
-              @options[:pr_title],
-              @options[:message]
+              args[:source_branch],
+              args[:title],
+              args[:message]
              ).and_return({"html_url" => "http://example.com/pulls/22"})
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Submitted PR/).to_stdout
+      expect { @it.open_pull_request(**args) }.to output(/Submitted PR/).to_stdout
     end
 
     it 'skips submitting PR if one has already been issued' do
@@ -45,32 +50,31 @@
       }
 
       expect(@client).to receive(:pull_requests)
-        .with(@git_repo,
+        .with(args[:repo_path],
               :state => 'open',
               :base => 'master',
-              :head => "#{@namespace}:#{@options[:branch]}"
+              :head => "#{args[:namespace]}:#{args[:source_branch]}"
              ).and_return([pr])
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 PRs found for branch test/).to_stdout
+      expect { @it.open_pull_request(**args) }.to output("Skipped! 1 PRs found for branch 'test'\n").to_stdout
     end
 
     context 'when labels are set' do
-      it 'adds labels to PR' do
-        @options[:pr_labels] = "HELLO,WORLD"
+      let(:labels) { %w{HELLO WORLD} }
 
+      it 'adds labels to PR' do
         allow(@client).to receive(:create_pull_request).and_return({"html_url" => "http://example.com/pulls/22", "number" => "44"})
         allow(@client).to receive(:pull_requests)
-          .with(@git_repo,
+          .with(args[:repo_path],
                 :state => 'open',
                 :base => 'master',
-                :head => "#{@namespace}:#{@options[:branch]}"
+                :head => "#{args[:namespace]}:#{args[:source_branch]}"
                ).and_return([])
-
         expect(@client).to receive(:add_labels_to_an_issue)
-          .with(@git_repo,
+          .with(args[:repo_path],
                 "44",
                 ["HELLO", "WORLD"]
                )
-        expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
+        expect { @it.open_pull_request(**args) }.to output(/Attaching the following labels to PR 44: HELLO, WORLD/).to_stdout
       end
     end
   end
diff --git a/spec/unit/modulesync/git_service/gitlab_spec.rb b/spec/unit/modulesync/git_service/gitlab_spec.rb
index b8f05746..d50181c6 100644
--- a/spec/unit/modulesync/git_service/gitlab_spec.rb
+++ b/spec/unit/modulesync/git_service/gitlab_spec.rb
@@ -5,38 +5,43 @@
 describe ModuleSync::GitService::GitLab do
   context '::open_pull_request' do
     before(:each) do
-      @git_repo = 'test/modulesync'
-      @namespace, @repo_name = @git_repo.split('/')
-      @options = {
-        :pr => true,
-        :pr_title => 'Test MR is submitted',
-        :branch => 'test',
-        :message => 'Hello world',
-        :pr_auto_merge => false,
-      }
-
       @client = double()
       allow(Gitlab::Client).to receive(:new).and_return(@client)
       @it = ModuleSync::GitService::GitLab.new('test', 'https://gitlab.com/api/v4')
     end
 
+    let(:args) do
+      {
+        repo_path: 'test/modulesync',
+        namespace: 'test',
+        title: 'Test MR is submitted',
+        message: 'Hello world',
+        source_branch: 'test',
+        target_branch: 'master',
+        labels: labels,
+        noop: false,
+      }
+    end
+
+    let(:labels) { [] }
+
     it 'submits MR when --pr is set' do
       allow(@client).to receive(:merge_requests)
-        .with(@git_repo,
+        .with(args[:repo_path],
               :state => 'opened',
-              :source_branch => "#{@namespace}:#{@options[:branch]}",
+              :source_branch => "#{args[:namespace]}:#{args[:source_branch]}",
               :target_branch => 'master',
              ).and_return([])
 
       expect(@client).to receive(:create_merge_request)
-        .with(@git_repo,
-              @options[:pr_title],
+        .with(args[:repo_path],
+              args[:title],
               :labels => [],
-              :source_branch => @options[:branch],
+              :source_branch => args[:source_branch],
               :target_branch => 'master',
              ).and_return({"html_url" => "http://example.com/pulls/22"})
 
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Submitted MR/).to_stdout
+      expect { @it.open_pull_request(**args) }.to output(/Submitted MR/).to_stdout
     end
 
     it 'skips submitting MR if one has already been issued' do
@@ -47,38 +52,38 @@
       }
 
       expect(@client).to receive(:merge_requests)
-        .with(@git_repo,
+        .with(args[:repo_path],
               :state => 'opened',
-              :source_branch => "#{@namespace}:#{@options[:branch]}",
+              :source_branch => "#{args[:namespace]}:#{args[:source_branch]}",
               :target_branch => 'master',
              ).and_return([mr])
 
-      expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Skipped! 1 MRs found for branch test/).to_stdout
+      expect { @it.open_pull_request(**args) }.to output("Skipped! 1 MRs found for branch 'test'\n").to_stdout
     end
 
-
     context 'when labels are set' do
+      let(:labels) { %w{HELLO WORLD} }
+
       it 'adds labels to MR' do
-        @options[:pr_labels] = "HELLO,WORLD"
         mr = double()
         allow(mr).to receive(:iid).and_return("42")
 
         expect(@client).to receive(:create_merge_request)
-          .with(@git_repo,
-                @options[:pr_title],
+          .with(args[:repo_path],
+                args[:title],
                 :labels => ["HELLO", "WORLD"],
-                :source_branch => @options[:branch],
+                :source_branch => args[:source_branch],
                 :target_branch => 'master',
                ).and_return(mr)
 
         allow(@client).to receive(:merge_requests)
-          .with(@git_repo,
+          .with(args[:repo_path],
                 :state => 'opened',
-                :source_branch => "#{@namespace}:#{@options[:branch]}",
+                :source_branch => "#{args[:namespace]}:#{args[:source_branch]}",
                 :target_branch => 'master',
                ).and_return([])
 
-        expect { @it.open_pull_request(@namespace, @repo_name, @options) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
+        expect { @it.open_pull_request(**args) }.to output(/Attached the following labels to MR 42: HELLO, WORLD/).to_stdout
       end
     end
   end

From 51cb45a726c789761e2dcf93ff52623bb330a1dd Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 13:52:12 +0100
Subject: [PATCH 10/21] CLI: Fix PR default target branch

Thanks to previous commit, this commit fixes #207 and now honors the
default repo branch.
---
 lib/modulesync/cli.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/modulesync/cli.rb b/lib/modulesync/cli.rb
index bd733e67..9a90e52b 100644
--- a/lib/modulesync/cli.rb
+++ b/lib/modulesync/cli.rb
@@ -104,7 +104,7 @@ class Base < Thor
              :default => CLI.defaults[:pr_labels] || []
       option :pr_target_branch,
              :desc => 'Target branch for the pull/merge request',
-             :default => CLI.defaults[:pr_target_branch] || 'master'
+             :default => CLI.defaults[:pr_target_branch]
       option :offline,
              :type => :boolean,
              :desc => 'Do not run any Git commands. Allows the user to manage Git outside of ModuleSync.',

From d0359414408d8f716ffe7d4bd330c479f7670a68 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 1 Jan 2021 11:48:31 +0100
Subject: [PATCH 11/21] PR/MR: Output PR/MR intent only when relevent in noop

This commit removes the outputs like "Would submit PR" when there are no
changes after update (in no-op mode).
---
 features/update/pull_request.feature | 82 ++++++++++++++++++++++++++--
 lib/modulesync.rb                    |  4 +-
 lib/modulesync/repository.rb         |  2 +
 3 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 98f82e1b..b3572d1a 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -8,10 +8,19 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         github: {}
       """
-    And a directory named "moduleroot"
     And I set the environment variables to:
       | variable     | value  |
       | GITHUB_TOKEN | foobar |
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     When I run `msync update --noop --branch managed_update --pr`
     Then the output should contain "Would submit PR "
     And the exit status should be 0
@@ -19,7 +28,6 @@ Feature: Create a pull-request/merge-request after update
 
   Scenario: Run update in no-op mode and ask for GitLab MR
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
-    And a directory named "moduleroot"
     And a file named "managed_modules.yml" with:
       """
       ---
@@ -29,11 +37,38 @@ Feature: Create a pull-request/merge-request after update
     And I set the environment variables to:
       | variable     | value  |
       | GITLAB_TOKEN | foobar |
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     When I run `msync update --noop --branch managed_update --pr`
     Then the output should contain "Would submit MR "
     And the exit status should be 0
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 
+  Scenario: Run update without changes in no-op mode and ask for GitLab MR
+    Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
+    And a directory named "moduleroot"
+    And a file named "managed_modules.yml" with:
+      """
+      ---
+      puppet-test:
+        gitlab: {}
+      """
+    And I set the environment variables to:
+      | variable     | value  |
+      | GITLAB_TOKEN | foobar |
+    When I run `msync update --noop --branch managed_update --pr`
+    Then the output should not contain "Would submit MR "
+    And the exit status should be 0
+    And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
+
   Scenario: Ask for PR without credentials
     Given a basic setup with a puppet module "puppet-test" from "fakenamespace"
     And a file named "managed_modules.yml" with:
@@ -42,7 +77,16 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         gitlab: {}
       """
-    And a directory named "moduleroot"
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     When I run `msync update --noop --pr`
     Then the stderr should contain "No GitLab token specified to create a merge request"
     And the exit status should be 1
@@ -61,7 +105,16 @@ Feature: Create a pull-request/merge-request after update
         gitlab:
           token: 'secret'
       """
-    And a directory named "moduleroot"
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     When I run `msync update --noop --branch managed_update --pr`
     Then the exit status should be 0
     And the output should contain "Would submit PR "
@@ -78,7 +131,16 @@ Feature: Create a pull-request/merge-request after update
         gitlab:
           token: 'secret'
       """
-    And a directory named "moduleroot"
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     When I run `msync update --noop --branch managed_update --pr --pr-target-branch managed_update`
     Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'managed_update'"
     And the exit status should be 1
@@ -94,6 +156,16 @@ Feature: Create a pull-request/merge-request after update
         github:
           token: 'secret'
       """
+    And a file named "config_defaults.yml" with:
+      """
+      ---
+      test:
+        name: aruba
+      """
+    And a file named "moduleroot/test.erb" with:
+      """
+      <%= @configs['name'] %>
+      """
     And a directory named "moduleroot"
     When I run `msync update --noop --pr`
     Then the stderr should contain "Unable to open a pull request with the same source and target branch: 'custom_default_branch'"
diff --git a/lib/modulesync.rb b/lib/modulesync.rb
index 31733ce7..0fa29971 100644
--- a/lib/modulesync.rb
+++ b/lib/modulesync.rb
@@ -129,8 +129,8 @@ def self.manage_module(puppet_module, module_files, defaults)
 
     if options[:noop]
       puts "Using no-op. Files in '#{puppet_module.given_name}' may be changed but will not be committed."
-      puppet_module.repository.show_changes(options)
-      options[:pr] && puppet_module.open_pull_request
+      changed = puppet_module.repository.show_changes(options)
+      changed && options[:pr] && puppet_module.open_pull_request
     elsif !options[:offline]
       pushed = puppet_module.repository.submit_changes(files_to_manage, options)
       # Only bump/tag if pushing didn't fail (i.e. there were changes)
diff --git a/lib/modulesync/repository.rb b/lib/modulesync/repository.rb
index 31fdcf7f..8e3e43e2 100644
--- a/lib/modulesync/repository.rb
+++ b/lib/modulesync/repository.rb
@@ -157,6 +157,8 @@ def show_changes(options)
 
       puts "\n\n"
       puts '--------------------------------'
+
+      git.diff('HEAD', '--').any? || untracked_unignored_files.any?
     end
   end
 end

From b57f9e258cb26306db22db96fb4a949f3d9da541 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 21 Apr 2021 22:40:29 +0200
Subject: [PATCH 12/21] Tests: Fix PR/MR feature tests

At this point, GitService should guess "git service" type (ie. GitHub or
GitLab) and rely on multiple sources to do.

As there is no more default (ie. which was GitHub) and the local git
repo used does not provide guessable service type, we need to add this
`base_url` options on each tests before
---
 features/update/pull_request.feature | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index b3572d1a..439f04e1 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -9,8 +9,9 @@ Feature: Create a pull-request/merge-request after update
         github: {}
       """
     And I set the environment variables to:
-      | variable     | value  |
-      | GITHUB_TOKEN | foobar |
+      | variable        | value                    |
+      | GITHUB_TOKEN    | foobar                   |
+      | GITHUB_BASE_URL | https://github.faker.com |
     And a file named "config_defaults.yml" with:
       """
       ---
@@ -32,7 +33,9 @@ Feature: Create a pull-request/merge-request after update
       """
       ---
       puppet-test:
-        gitlab: {}
+        gitlab: {
+          base_url: 'https://gitlab.faker.com'
+      }
       """
     And I set the environment variables to:
       | variable     | value  |
@@ -59,7 +62,9 @@ Feature: Create a pull-request/merge-request after update
       """
       ---
       puppet-test:
-        gitlab: {}
+        gitlab: {
+          base_url: 'https://gitlab.faker.com'
+        }
       """
     And I set the environment variables to:
       | variable     | value  |
@@ -75,7 +80,9 @@ Feature: Create a pull-request/merge-request after update
       """
       ---
       puppet-test:
-        gitlab: {}
+        gitlab: {
+          base_url: https://gitlab.faker.com
+        }
       """
     And a file named "config_defaults.yml" with:
       """
@@ -100,9 +107,11 @@ Feature: Create a pull-request/merge-request after update
       ---
       puppet-github:
         github:
+          base_url: https://github.faker.com
           token: 'secret'
       puppet-gitlab:
         gitlab:
+          base_url: https://gitlab.faker.com
           token: 'secret'
       """
     And a file named "config_defaults.yml" with:
@@ -130,6 +139,7 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         gitlab:
           token: 'secret'
+          base_url: 'https://gitlab.faker.com'
       """
     And a file named "config_defaults.yml" with:
       """
@@ -155,6 +165,7 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         github:
           token: 'secret'
+          base_url: 'https://gitlab.faker.com'
       """
     And a file named "config_defaults.yml" with:
       """

From 44b6260e5ead0c491a38632b9e6b4ac757198356 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Thu, 31 Dec 2020 21:26:50 +0100
Subject: [PATCH 13/21] Rework PR/MR related code (2/2)

This commit implements a method to know which Git service (ie. GitHub or
GitLab) is used to host source code.

 * introduces a specific error when creadentials are missing
 * adds method to guess git service type based on a SourceCode instance
 * adds method to find with git service's endpoint should be used
 * adds method to find token
 * adds a method to pack a configuration hash (type, endpoint, token)
   based on a SourceCode instance
---
 features/update/pull_request.feature     |   2 +-
 lib/modulesync/git_service.rb            | 115 ++++++++++++++++-
 lib/modulesync/source_code.rb            |   3 +-
 spec/unit/modulesync/git_service_spec.rb | 156 ++++++++++++++++++++++-
 4 files changed, 265 insertions(+), 11 deletions(-)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 439f04e1..5b80156c 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -95,7 +95,7 @@ Feature: Create a pull-request/merge-request after update
       <%= @configs['name'] %>
       """
     When I run `msync update --noop --pr`
-    Then the stderr should contain "No GitLab token specified to create a merge request"
+    Then the stderr should contain "A token is require to use services from gitlab"
     And the exit status should be 1
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 
diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
index 1a512939..e653c7ff 100644
--- a/lib/modulesync/git_service.rb
+++ b/lib/modulesync/git_service.rb
@@ -1,24 +1,125 @@
 module ModuleSync
+  class Error < StandardError; end
+
   # Namespace for Git service classes (ie. GitHub, GitLab)
   module GitService
-    def self.instantiate(type:, options:)
-      options ||= {}
+    class MissingCredentialsError < Error; end
+
+    def self.instantiate(type:, endpoint:, token:)
       case type
       when :github
-        endpoint = options[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
-        token = options[:token] || ENV['GITHUB_TOKEN']
         raise ModuleSync::Error, 'No GitHub token specified to create a pull request' if token.nil?
         require 'modulesync/git_service/github'
         ModuleSync::GitService::GitHub.new(token, endpoint)
       when :gitlab
-        endpoint = options[:base_url] || ENV.fetch('GITLAB_BASE_URL', 'https://gitlab.com/api/v4')
-        token = options[:token] || ENV['GITLAB_TOKEN']
         raise ModuleSync::Error, 'No GitLab token specified to create a merge request' if token.nil?
         require 'modulesync/git_service/gitlab'
         ModuleSync::GitService::GitLab.new(token, endpoint)
       else
-        raise ModuleSync::Error, "Unable to manage a PR/MR for Git service: '#{type}'"
+        raise NotImplementedError, "Unable to manage a PR/MR for Git service: '#{type}'"
       end
     end
+
+    def self.configuration_for(sourcecode:)
+      type = type_for(sourcecode: sourcecode)
+
+      {
+        type: type,
+        endpoint: endpoint_for(sourcecode: sourcecode, type: type),
+        token: token_for(sourcecode: sourcecode, type: type),
+      }
+    end
+
+    # This method attempts to guess git service's type (ie. gitlab or github)
+    # It process in this order
+    #   1. use module specific configuration entry (ie. a specific entry named `gitlab` or `github`)
+    #   2. guess using remote url (ie. looking for `github` or `gitlab` string)
+    #   3. use environment variables (ie. check if GITHUB_TOKEN or GITLAB_TOKEN is set)
+    #   4. fail
+    def self.type_for(sourcecode:)
+      return :github unless sourcecode.options[:github].nil?
+      return :gitlab unless sourcecode.options[:gitlab].nil?
+      return :github if sourcecode.repository_remote.include? 'github'
+      return :gitlab if sourcecode.repository_remote.include? 'gitlab'
+
+      if ENV['GITLAB_TOKEN'].nil? && ENV['GITHUB_TOKEN'].nil?
+        raise ModuleSync::Error, <<~MESSAGE
+          Unable to guess Git service type without GITLAB_TOKEN or GITHUB_TOKEN sets.
+          MESSAGE
+      end
+
+      unless ENV['GITLAB_TOKEN'].nil? || ENV['GITHUB_TOKEN'].nil?
+        raise ModuleSync::Error, <<~MESSAGE
+          Unable to guess Git service type with both GITLAB_TOKEN and GITHUB_TOKEN sets.
+
+          Please set the wanted one in configuration (ie. add `gitlab:` or `github:` key)
+          MESSAGE
+      end
+
+      return :github unless ENV['GITHUB_TOKEN'].nil?
+      return :gitlab unless ENV['GITLAB_TOKEN'].nil?
+
+      raise NotImplementedError
+    end
+
+    # This method attempts to find git service's endpoint based on sourcecode and type
+    # It process in this order
+    #   1. use module specific configuration (ie. `base_url`)
+    #   2. use environment variable dependending on type (e.g. GITLAB_BASE_URL)
+    #   3. guess using the git remote url
+    #   4. fail
+    def self.endpoint_for(sourcecode:, type:)
+      endpoint = sourcecode.options.dig(type, :base_url)
+
+      endpoint ||= case type
+                   when :github
+                     ENV['GITHUB_BASE_URL']
+                   when :gitlab
+                     ENV['GITLAB_BASE_URL']
+                   end
+
+      endpoint ||= guess_endpoint_from(remote: sourcecode.repository_remote, type: type)
+
+      raise NotImplementedError, <<~MESSAGE if endpoint.nil?
+        Unable to guess endpoint for remote: '#{sourcecode.repository_remote}'
+        Please provide `base_url` option in configuration file
+      MESSAGE
+
+      endpoint
+    end
+
+    # This method attempts to guess the git service endpoint based on remote and type
+    # FIXME: It only supports "git@hostname:repo_path" scheme
+    def self.guess_endpoint_from(remote:, type:)
+      pattern = /^git@(.*):(.*)(\.git)*$/
+      return nil unless remote.match?(pattern)
+
+      endpoint = remote.sub(pattern, 'https://\1')
+      endpoint += '/api/v4' if type == :gitlab
+      endpoint
+    end
+
+    # This method attempts to find the token associated to provided sourcecode and type
+    # It process in this order:
+    #   1. use module specific configuration (ie. `token`)
+    #   2. use environment variable depending on type (e.g. GITLAB_TOKEN)
+    #   3. fail
+    def self.token_for(sourcecode:, type:)
+      token = sourcecode.options.dig(type, :token)
+
+      token ||= case type
+                when :github
+                  ENV['GITHUB_TOKEN']
+                when :gitlab
+                  ENV['GITLAB_TOKEN']
+                end
+
+      raise MissingCredentialsError, <<~MESSAGE if token.nil?
+        A token is require to use services from #{type}:
+          Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options.
+        MESSAGE
+
+      token
+    end
   end
 end
diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb
index ff487c5d..10d72633 100644
--- a/lib/modulesync/source_code.rb
+++ b/lib/modulesync/source_code.rb
@@ -49,7 +49,8 @@ def path(*parts)
     end
 
     def open_pull_request
-      git_service = GitService.instantiate type: git_service_type, options: @options[git_service_type]
+      git_service_options = GitService.configuration_for(sourcecode: self)
+      git_service = GitService.instantiate(**git_service_options)
       git_service.open_pull_request(
         repo_path: repository_path,
         namespace: repository_namespace,
diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb
index eee64366..d4b115d8 100644
--- a/spec/unit/modulesync/git_service_spec.rb
+++ b/spec/unit/modulesync/git_service_spec.rb
@@ -1,15 +1,167 @@
 require 'modulesync/git_service'
 
 describe ModuleSync::GitService do
+  before do
+    options = ModuleSync.config_defaults.merge({
+      git_base: 'file:///tmp/dummy',
+    })
+    ModuleSync.instance_variable_set '@options', options
+  end
+
   context 'when instantiate a GitHub service without credentials' do
     it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :github, options: nil) }.to raise_error(ModuleSync::Error, 'No GitHub token specified to create a pull request')
+      expect { ModuleSync::GitService.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::Error, 'No GitHub token specified to create a pull request')
     end
   end
 
   context 'when instantiate a GitLab service without credentials' do
     it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :gitlab, options: nil) }.to raise_error(ModuleSync::Error, 'No GitLab token specified to create a merge request')
+      expect { ModuleSync::GitService.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::Error, 'No GitLab token specified to create a merge request')
+    end
+  end
+
+  context 'when guessing the git service configuration' do
+    before do
+      allow(ENV).to receive(:[])
+        .and_return(nil)
+    end
+
+    let(:sourcecode) do
+      ModuleSync::SourceCode.new 'puppet-test', sourcecode_options
+    end
+
+    context 'when using a complete git service configuration entry' do
+      let(:sourcecode_options) do
+        {
+          gitlab: {
+            base_url: 'https://vcs.example.com/api/v4',
+            token: 'secret',
+          },
+        }
+      end
+
+      it 'build git service arguments from configuration entry' do
+        expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+          type: :gitlab,
+          endpoint: 'https://vcs.example.com/api/v4',
+          token: 'secret',
+        })
+      end
+    end
+
+    context 'when using a simple git service key entry' do
+      let(:sourcecode_options) do
+        {
+          gitlab: {},
+          remote: 'git@git.example.com:namespace/puppet-test',
+        }
+      end
+
+      context 'with GITLAB_BASE_URL and GITLAB_TOKEN environment variables sets' do
+        it 'build git service arguments from environment variables' do
+          allow(ENV).to receive(:[])
+            .with('GITLAB_BASE_URL')
+            .and_return('https://vcs.example.com/api/v4')
+          allow(ENV).to receive(:[])
+            .with('GITLAB_TOKEN')
+            .and_return('secret')
+
+          expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+            type: :gitlab,
+            endpoint: 'https://vcs.example.com/api/v4',
+            token: 'secret',
+          })
+        end
+      end
+
+      context 'with only GITLAB_TOKEN environment variable sets' do
+        it 'guesses the endpoint based on repository remote' do
+          allow(ENV).to receive(:[])
+            .with('GITLAB_TOKEN')
+            .and_return('secret')
+
+          expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+            type: :gitlab,
+            endpoint: 'https://git.example.com/api/v4',
+            token: 'secret',
+          })
+        end
+      end
+
+      context 'without any environment variable sets' do
+        it 'raises an error about missing credential' do
+          expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)}
+            .to raise_error ModuleSync::GitService::MissingCredentialsError
+        end
+      end
+    end
+
+    context 'without git service configuration entry' do
+      context 'with a guessable endpoint based on repository remote' do
+        let(:sourcecode_options) do
+          {
+            remote: 'git@gitlab.example.com:namespace/puppet-test',
+          }
+        end
+
+        context 'with a GITLAB_TOKEN environment variable sets' do
+          it 'guesses git service configuration' do
+            allow(ENV).to receive(:[])
+              .with('GITLAB_TOKEN')
+              .and_return('secret')
+
+            expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+              type: :gitlab,
+              endpoint: 'https://gitlab.example.com/api/v4',
+              token: 'secret',
+            })
+          end
+        end
+
+        context 'without a GITLAB_TOKEN environment variable sets' do
+          it 'raise an error about missing credential' do
+            expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)}
+              .to raise_error ModuleSync::Error
+          end
+        end
+      end
+
+      context 'with a unguessable endpoint' do
+        let(:sourcecode_options) do
+          {
+            remote: 'git@vcs.example.com:namespace/puppet-test',
+          }
+        end
+
+        context 'with GITHUB_TOKEN environments variable sets' do
+          it 'guesses git service configuration' do
+            allow(ENV).to receive(:[])
+              .with('GITHUB_TOKEN')
+              .and_return('secret')
+
+            expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+              type: :github,
+              endpoint: 'https://vcs.example.com',
+              token: 'secret',
+            })
+          end
+        end
+
+        context 'with GITLAB_TOKEN and GITHUB_TOKEN environments variables sets' do
+          it 'raises an error' do
+            allow(ENV).to receive(:[])
+              .with('GITHUB_TOKEN')
+              .and_return('secret')
+
+            allow(ENV).to receive(:[])
+              .with('GITLAB_TOKEN')
+              .and_return('secret')
+
+            expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)}
+              .to raise_error ModuleSync::Error
+          end
+        end
+      end
     end
   end
 end

From 3fbe1b55e7d69e26af7a16c5d88f5e7ace9d7180 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 23 Apr 2021 16:48:23 +0200
Subject: [PATCH 14/21] Rubocop: Update autogenerated todo file

---
 .rubocop_todo.yml | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index b69b2939..640d7f2b 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,37 +1,43 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2021-04-22 16:30:35 +0200 using RuboCop version 0.50.0.
+# on 2021-04-23 16:47:47 +0200 using RuboCop version 0.50.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 1
+# Offense count: 3
 Lint/UselessAssignment:
   Exclude:
     - 'lib/modulesync.rb'
 
 # Offense count: 10
 Metrics/AbcSize:
-  Max: 67
+  Max: 59
 
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 128
+  Max: 125
 
-# Offense count: 3
+# Offense count: 4
 Metrics/CyclomaticComplexity:
-  Max: 12
+  Max: 13
+
+# Offense count: 3
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# URISchemes: http, https
+Metrics/LineLength:
+  Max: 186
 
-# Offense count: 13
+# Offense count: 17
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
-  Max: 36
+  Max: 32
 
 # Offense count: 3
 Metrics/PerceivedComplexity:
-  Max: 13
+  Max: 14
 
 # Offense count: 8
 Style/Documentation:
@@ -49,3 +55,12 @@ Style/Documentation:
 Style/EachWithObject:
   Exclude:
     - 'lib/modulesync/util.rb'
+
+# Offense count: 2
+# Cop supports --auto-correct.
+# Configuration parameters: EnforcedStyleForMultiline, SupportedStylesForMultiline.
+# SupportedStylesForMultiline: comma, consistent_comma, no_comma
+Style/TrailingCommaInArguments:
+  Exclude:
+    - 'lib/modulesync/git_service/base.rb'
+    - 'lib/modulesync/source_code.rb'

From a476c950bb8d4485ffa5aef94b7aad3322d7967f Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Fri, 23 Apr 2021 15:20:44 +0200
Subject: [PATCH 15/21] Spec: Add a minimal spec file for SourceCode

---
 spec/unit/modulesync/source_code_spec.rb | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 spec/unit/modulesync/source_code_spec.rb

diff --git a/spec/unit/modulesync/source_code_spec.rb b/spec/unit/modulesync/source_code_spec.rb
new file mode 100644
index 00000000..8317aeb9
--- /dev/null
+++ b/spec/unit/modulesync/source_code_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe ModuleSync::SourceCode do
+  before do
+    options = ModuleSync.config_defaults.merge({
+      git_base: 'file:///tmp/dummy',
+    })
+    ModuleSync.instance_variable_set '@options', options
+  end
+
+  subject do
+    ModuleSync::SourceCode.new('namespace/name', nil)
+  end
+
+  it 'has a repository namespace sets to "namespace"' do
+    expect(subject.repository_namespace).to eq 'namespace'
+  end
+
+  it 'has a repository name sets to "name"' do
+    expect(subject.repository_name).to eq 'name'
+  end
+end

From ce795d52adcf670656b661e0899a4b554c01b00a Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Tue, 5 Oct 2021 21:45:24 +0200
Subject: [PATCH 16/21] GitService: Improve hostname extraction

This commit adds support for more repository notations and provides unit
tests.
---
 lib/modulesync/git_service.rb            | 33 ++++++++++++++++---
 lib/modulesync/source_code.rb            | 10 ++++--
 spec/unit/modulesync/git_service_spec.rb | 40 ++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
index e653c7ff..cee3506a 100644
--- a/lib/modulesync/git_service.rb
+++ b/lib/modulesync/git_service.rb
@@ -89,12 +89,11 @@ def self.endpoint_for(sourcecode:, type:)
     end
 
     # This method attempts to guess the git service endpoint based on remote and type
-    # FIXME: It only supports "git@hostname:repo_path" scheme
     def self.guess_endpoint_from(remote:, type:)
-      pattern = /^git@(.*):(.*)(\.git)*$/
-      return nil unless remote.match?(pattern)
+      hostname = extract_hostname(remote)
+      return nil if hostname.nil?
 
-      endpoint = remote.sub(pattern, 'https://\1')
+      endpoint = "https://#{hostname}"
       endpoint += '/api/v4' if type == :gitlab
       endpoint
     end
@@ -121,5 +120,31 @@ def self.token_for(sourcecode:, type:)
 
       token
     end
+
+    # This method extracts hostname from URL like:
+    #
+    # - ssh://[user@]host.xz[:port]/path/to/repo.git/
+    # - git://host.xz[:port]/path/to/repo.git/
+    # - [user@]host.xz:path/to/repo.git/
+    # - http[s]://host.xz[:port]/path/to/repo.git/
+    # - ftp[s]://host.xz[:port]/path/to/repo.git/
+    #
+    # Returns nil if
+    # - /path/to/repo.git/
+    # - file:///path/to/repo.git/
+    # - any invalid URL
+    def self.extract_hostname(url)
+      #pattern = /^((?<user>.*)@)*(?<hostname>.*):(?<path>[a-zA-Z].*)*$/
+      return nil if url.start_with?('/') || url.start_with?('file://') # local path (e.g. file:///path/to/repo)
+
+      unless url.start_with?(/[a-z]+:\/\//) # SSH notation does not contain protocol (e.g. user@server:path/to/repo/)
+        pattern = /^(.*@)?(?<hostname>[\w|.]*):(.*)$/ # SSH path (e.g. user@server:repo)
+        return url.match(pattern)[:hostname] if url.match?(pattern)
+      end
+
+      return URI.parse(url).host
+    rescue URI::InvalidURIError => e
+      return nil
+    end
   end
 end
diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb
index 10d72633..248bd420 100644
--- a/lib/modulesync/source_code.rb
+++ b/lib/modulesync/source_code.rb
@@ -48,9 +48,15 @@ def path(*parts)
       File.join(working_directory, *parts)
     end
 
+    def git_service
+      @git_service ||= GitService.instantiate(**git_service_configuration)
+    end
+
+    def git_service_configuration
+      @git_service_configuration ||= GitService.configuration_for(sourcecode: self)
+    end
+
     def open_pull_request
-      git_service_options = GitService.configuration_for(sourcecode: self)
-      git_service = GitService.instantiate(**git_service_options)
       git_service.open_pull_request(
         repo_path: repository_path,
         namespace: repository_namespace,
diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb
index d4b115d8..9f372363 100644
--- a/spec/unit/modulesync/git_service_spec.rb
+++ b/spec/unit/modulesync/git_service_spec.rb
@@ -1,3 +1,4 @@
+require 'modulesync'
 require 'modulesync/git_service'
 
 describe ModuleSync::GitService do
@@ -164,4 +165,43 @@
       end
     end
   end
+
+  RSpec.shared_examples 'hostname_extractor' do |url, hostname|
+    context "with '#{url}' URL" do
+      subject { ModuleSync::GitService.extract_hostname(url) }
+      it "should extract '#{hostname}' as hostname" do
+        expect(subject).to eq(hostname)
+      end
+    end
+  end
+
+  context '#extract_hostname' do
+    [
+      %w[ssh://user@host.xz:4444/path/to/repo.git/ host.xz],
+      %w[ssh://user@host.xz:/path/to/repo.git/     host.xz],
+      %w[ssh://host.xz/path/to/repo.git/           host.xz],
+
+      %w[git://host.xz/path/to/repo.git/           host.xz],
+      %w[git://host.xz/path/to/repo/               host.xz],
+      %w[git://host.xz/path/to/repo                host.xz],
+
+      %w[user@host.xz:path/to/repo.git/            host.xz],
+      %w[user@host.xz:path/to/repo.git             host.xz],
+      %w[user@host.xz:path/to/repo                 host.xz],
+      %w[host.xz:path/to/repo.git/                 host.xz],
+
+      %w[https://host.xz:8443/path/to/repo.git/    host.xz],
+      %w[https://host.xz/path/to/repo.git/         host.xz],
+
+      %w[ftp://host.xz/path/to/repo/               host.xz],
+
+      ['/path/to/repo.git/',                       nil],
+
+      ['file:///path/to/repo.git/',                nil],
+
+      ['something-invalid',                        nil],
+    ].each do |url, hostname|
+      it_should_behave_like 'hostname_extractor', url, hostname
+    end
+  end
 end

From bb02241d24bbb1e18a7b27f04f0dd33a697970d1 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 6 Oct 2021 00:54:45 +0200
Subject: [PATCH 17/21] GitService: Raises error about missing credentials only
 on instantiation

---
 features/update/pull_request.feature     |  2 +-
 lib/modulesync/git_service.rb            | 12 +++++-------
 spec/unit/modulesync/git_service_spec.rb | 22 ++++++++++++++--------
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 5b80156c..9ba2912e 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -95,7 +95,7 @@ Feature: Create a pull-request/merge-request after update
       <%= @configs['name'] %>
       """
     When I run `msync update --noop --pr`
-    Then the stderr should contain "A token is require to use services from gitlab"
+    Then the stderr should contain "A token is required to use services from gitlab"
     And the exit status should be 1
     And the puppet module "puppet-test" from "fakenamespace" should have no commits made by "Aruba"
 
diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
index cee3506a..7146ea75 100644
--- a/lib/modulesync/git_service.rb
+++ b/lib/modulesync/git_service.rb
@@ -6,13 +6,16 @@ module GitService
     class MissingCredentialsError < Error; end
 
     def self.instantiate(type:, endpoint:, token:)
+      raise MissingCredentialsError, <<~MESSAGE if token.nil?
+        A token is required to use services from #{type}:
+          Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options.
+      MESSAGE
+
       case type
       when :github
-        raise ModuleSync::Error, 'No GitHub token specified to create a pull request' if token.nil?
         require 'modulesync/git_service/github'
         ModuleSync::GitService::GitHub.new(token, endpoint)
       when :gitlab
-        raise ModuleSync::Error, 'No GitLab token specified to create a merge request' if token.nil?
         require 'modulesync/git_service/gitlab'
         ModuleSync::GitService::GitLab.new(token, endpoint)
       else
@@ -113,11 +116,6 @@ def self.token_for(sourcecode:, type:)
                   ENV['GITLAB_TOKEN']
                 end
 
-      raise MissingCredentialsError, <<~MESSAGE if token.nil?
-        A token is require to use services from #{type}:
-          Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options.
-        MESSAGE
-
       token
     end
 
diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb
index 9f372363..d8dbd619 100644
--- a/spec/unit/modulesync/git_service_spec.rb
+++ b/spec/unit/modulesync/git_service_spec.rb
@@ -11,13 +11,13 @@
 
   context 'when instantiate a GitHub service without credentials' do
     it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::Error, 'No GitHub token specified to create a pull request')
+      expect { ModuleSync::GitService.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
     end
   end
 
   context 'when instantiate a GitLab service without credentials' do
     it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::Error, 'No GitLab token specified to create a merge request')
+      expect { ModuleSync::GitService.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
     end
   end
 
@@ -90,9 +90,12 @@
       end
 
       context 'without any environment variable sets' do
-        it 'raises an error about missing credential' do
-          expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)}
-            .to raise_error ModuleSync::GitService::MissingCredentialsError
+        it 'returns a nil token' do
+          expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+            type: :gitlab,
+            endpoint: 'https://git.example.com/api/v4',
+            token: nil,
+          })
         end
       end
     end
@@ -120,9 +123,12 @@
         end
 
         context 'without a GITLAB_TOKEN environment variable sets' do
-          it 'raise an error about missing credential' do
-            expect{ModuleSync::GitService.configuration_for(sourcecode: sourcecode)}
-              .to raise_error ModuleSync::Error
+          it 'returns a nil token' do
+            expect(ModuleSync::GitService.configuration_for(sourcecode: sourcecode)).to eq({
+              type: :gitlab,
+              endpoint: 'https://gitlab.example.com/api/v4',
+              token: nil,
+            })
           end
         end
       end

From ba59519e64e9922b20210f7f364d61165fc1474b Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 6 Oct 2021 14:52:53 +0200
Subject: [PATCH 18/21] GitService: Move factory part into dedicated module

---
 lib/modulesync/git_service.rb                 | 18 ---------------
 lib/modulesync/git_service/factory.rb         | 23 +++++++++++++++++++
 lib/modulesync/source_code.rb                 |  3 ++-
 .../modulesync/git_service/factory_spec.rb    | 16 +++++++++++++
 spec/unit/modulesync/git_service_spec.rb      | 12 ----------
 5 files changed, 41 insertions(+), 31 deletions(-)
 create mode 100644 lib/modulesync/git_service/factory.rb
 create mode 100644 spec/unit/modulesync/git_service/factory_spec.rb

diff --git a/lib/modulesync/git_service.rb b/lib/modulesync/git_service.rb
index 7146ea75..df606bc9 100644
--- a/lib/modulesync/git_service.rb
+++ b/lib/modulesync/git_service.rb
@@ -5,24 +5,6 @@ class Error < StandardError; end
   module GitService
     class MissingCredentialsError < Error; end
 
-    def self.instantiate(type:, endpoint:, token:)
-      raise MissingCredentialsError, <<~MESSAGE if token.nil?
-        A token is required to use services from #{type}:
-          Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options.
-      MESSAGE
-
-      case type
-      when :github
-        require 'modulesync/git_service/github'
-        ModuleSync::GitService::GitHub.new(token, endpoint)
-      when :gitlab
-        require 'modulesync/git_service/gitlab'
-        ModuleSync::GitService::GitLab.new(token, endpoint)
-      else
-        raise NotImplementedError, "Unable to manage a PR/MR for Git service: '#{type}'"
-      end
-    end
-
     def self.configuration_for(sourcecode:)
       type = type_for(sourcecode: sourcecode)
 
diff --git a/lib/modulesync/git_service/factory.rb b/lib/modulesync/git_service/factory.rb
new file mode 100644
index 00000000..263ad842
--- /dev/null
+++ b/lib/modulesync/git_service/factory.rb
@@ -0,0 +1,23 @@
+module ModuleSync
+  module GitService
+    module Factory
+      def self.instantiate(type:, endpoint:, token:)
+        raise MissingCredentialsError, <<~MESSAGE if token.nil?
+        A token is required to use services from #{type}:
+          Please set environment variable: "#{type.upcase}_TOKEN" or set the token entry in module options.
+        MESSAGE
+
+        case type
+        when :github
+          require 'modulesync/git_service/github'
+          ModuleSync::GitService::GitHub.new(token, endpoint)
+        when :gitlab
+          require 'modulesync/git_service/gitlab'
+          ModuleSync::GitService::GitLab.new(token, endpoint)
+        else
+          raise NotImplementedError, "Unable to manage a PR/MR for Git service: '#{type}'"
+        end
+      end
+    end
+  end
+end
diff --git a/lib/modulesync/source_code.rb b/lib/modulesync/source_code.rb
index 248bd420..12cfeab7 100644
--- a/lib/modulesync/source_code.rb
+++ b/lib/modulesync/source_code.rb
@@ -1,5 +1,6 @@
 require 'modulesync'
 require 'modulesync/git_service'
+require 'modulesync/git_service/factory'
 require 'modulesync/repository'
 require 'modulesync/util'
 
@@ -49,7 +50,7 @@ def path(*parts)
     end
 
     def git_service
-      @git_service ||= GitService.instantiate(**git_service_configuration)
+      @git_service ||= GitService::Factory.instantiate(**git_service_configuration)
     end
 
     def git_service_configuration
diff --git a/spec/unit/modulesync/git_service/factory_spec.rb b/spec/unit/modulesync/git_service/factory_spec.rb
new file mode 100644
index 00000000..9e6234cc
--- /dev/null
+++ b/spec/unit/modulesync/git_service/factory_spec.rb
@@ -0,0 +1,16 @@
+require 'modulesync'
+require 'modulesync/git_service/factory'
+
+describe ModuleSync::GitService::Factory do
+  context 'when instantiate a GitHub service without credentials' do
+    it 'raises an error' do
+      expect { ModuleSync::GitService::Factory.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
+    end
+  end
+
+  context 'when instantiate a GitLab service without credentials' do
+    it 'raises an error' do
+      expect { ModuleSync::GitService::Factory.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
+    end
+  end
+end
diff --git a/spec/unit/modulesync/git_service_spec.rb b/spec/unit/modulesync/git_service_spec.rb
index d8dbd619..e8016c23 100644
--- a/spec/unit/modulesync/git_service_spec.rb
+++ b/spec/unit/modulesync/git_service_spec.rb
@@ -9,18 +9,6 @@
     ModuleSync.instance_variable_set '@options', options
   end
 
-  context 'when instantiate a GitHub service without credentials' do
-    it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :github, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
-    end
-  end
-
-  context 'when instantiate a GitLab service without credentials' do
-    it 'raises an error' do
-      expect { ModuleSync::GitService.instantiate(type: :gitlab, endpoint: nil, token: nil) }.to raise_error(ModuleSync::GitService::MissingCredentialsError)
-    end
-  end
-
   context 'when guessing the git service configuration' do
     before do
       allow(ENV).to receive(:[])

From 57a715a47aa28f088cff790882bdca3229703aaa Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 6 Oct 2021 15:18:37 +0200
Subject: [PATCH 19/21] Tests: Use example.com according to RFC2606

---
 features/update/pull_request.feature | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/features/update/pull_request.feature b/features/update/pull_request.feature
index 9ba2912e..c4762a5d 100644
--- a/features/update/pull_request.feature
+++ b/features/update/pull_request.feature
@@ -11,7 +11,7 @@ Feature: Create a pull-request/merge-request after update
     And I set the environment variables to:
       | variable        | value                    |
       | GITHUB_TOKEN    | foobar                   |
-      | GITHUB_BASE_URL | https://github.faker.com |
+      | GITHUB_BASE_URL | https://github.example.com |
     And a file named "config_defaults.yml" with:
       """
       ---
@@ -34,7 +34,7 @@ Feature: Create a pull-request/merge-request after update
       ---
       puppet-test:
         gitlab: {
-          base_url: 'https://gitlab.faker.com'
+          base_url: 'https://gitlab.example.com'
       }
       """
     And I set the environment variables to:
@@ -63,7 +63,7 @@ Feature: Create a pull-request/merge-request after update
       ---
       puppet-test:
         gitlab: {
-          base_url: 'https://gitlab.faker.com'
+          base_url: 'https://gitlab.example.com'
         }
       """
     And I set the environment variables to:
@@ -81,7 +81,7 @@ Feature: Create a pull-request/merge-request after update
       ---
       puppet-test:
         gitlab: {
-          base_url: https://gitlab.faker.com
+          base_url: https://gitlab.example.com
         }
       """
     And a file named "config_defaults.yml" with:
@@ -107,11 +107,11 @@ Feature: Create a pull-request/merge-request after update
       ---
       puppet-github:
         github:
-          base_url: https://github.faker.com
+          base_url: https://github.example.com
           token: 'secret'
       puppet-gitlab:
         gitlab:
-          base_url: https://gitlab.faker.com
+          base_url: https://gitlab.example.com
           token: 'secret'
       """
     And a file named "config_defaults.yml" with:
@@ -139,7 +139,7 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         gitlab:
           token: 'secret'
-          base_url: 'https://gitlab.faker.com'
+          base_url: 'https://gitlab.example.com'
       """
     And a file named "config_defaults.yml" with:
       """
@@ -165,7 +165,7 @@ Feature: Create a pull-request/merge-request after update
       puppet-test:
         github:
           token: 'secret'
-          base_url: 'https://gitlab.faker.com'
+          base_url: 'https://gitlab.example.com'
       """
     And a file named "config_defaults.yml" with:
       """

From a3bf16f97de3bee59c04f79e3980949c908963d8 Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 6 Oct 2021 15:19:54 +0200
Subject: [PATCH 20/21] GitService: Define a protected #_open_pull_request
 method

---
 lib/modulesync/git_service/base.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/modulesync/git_service/base.rb b/lib/modulesync/git_service/base.rb
index 81ef8109..154e00a6 100644
--- a/lib/modulesync/git_service/base.rb
+++ b/lib/modulesync/git_service/base.rb
@@ -19,6 +19,12 @@ def open_pull_request(repo_path:, namespace:, title:, message:, source_branch:,
           noop: noop,
         )
       end
+
+      protected
+
+      def _open_pull_request(repo_path:, namespace:, title:, message:, source_branch:, target_branch:, labels:, noop:) # rubocop:disable Metrics/ParameterLists, Metrics/LineLength
+        raise NotImplementedError
+      end
     end
   end
 end

From 57ebbb4e3f06b32fcca68aa23cf4001d87d3defe Mon Sep 17 00:00:00 2001
From: Romuald Conty <romuald@opus-codium.fr>
Date: Wed, 6 Oct 2021 15:20:48 +0200
Subject: [PATCH 21/21] GitServices: Clean up requirements

---
 lib/modulesync/git_service/github.rb | 2 --
 lib/modulesync/git_service/gitlab.rb | 4 +---
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/modulesync/git_service/github.rb b/lib/modulesync/git_service/github.rb
index a98c7848..eef84c87 100644
--- a/lib/modulesync/git_service/github.rb
+++ b/lib/modulesync/git_service/github.rb
@@ -1,8 +1,6 @@
 require 'modulesync/git_service'
 require 'modulesync/git_service/base'
-
 require 'octokit'
-require 'modulesync/util'
 
 module ModuleSync
   module GitService
diff --git a/lib/modulesync/git_service/gitlab.rb b/lib/modulesync/git_service/gitlab.rb
index 084ba99c..353f59cc 100644
--- a/lib/modulesync/git_service/gitlab.rb
+++ b/lib/modulesync/git_service/gitlab.rb
@@ -1,9 +1,7 @@
+require 'gitlab'
 require 'modulesync/git_service'
 require 'modulesync/git_service/base'
 
-require 'gitlab'
-require 'modulesync/util'
-
 module ModuleSync
   module GitService
     # GitLab creates and manages merge requests on gitlab.com or private GitLab