Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NoMethodError for --pr option (caused by module_options = nil) / introduce --noop #188

Merged
merged 5 commits into from
Aug 11, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions features/update.feature
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,28 @@ Feature: update
Then the output should not contain "error"
Then the output should not contain "rejected"

Scenario: Creating a GitHub PR with an update
Given a mocked git configuration
And a remote module repository
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

Scenario: Creating a GitLab MR with an update
Given a mocked git configuration
And a remote module repository
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

Scenario: Repository with a default branch other than master
Given a mocked git configuration
And a remote module repository with "develop" as the default branch
Expand Down
12 changes: 9 additions & 3 deletions lib/modulesync.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ def self.manage_module(puppet_module, module_files, module_options, defaults, op

if options[:noop]
Git.update_noop(git_repo, options)
options[:pr] && pr(module_options).manage(namespace, module_name, options)
elsif !options[:offline]
pushed = Git.update(git_repo, files_to_manage, options)
pushed && options[:pr] && pr(module_options).manage(namespace, module_name, options)
Expand Down Expand Up @@ -183,9 +184,14 @@ def self.update(options)
exit 1 if errors && options[:fail_on_warnings]
end

def self.pr(module_options = {})
github_conf = module_options[:github]
gitlab_conf = module_options[:gitlab]
def self.pr(module_options)
bittner marked this conversation as resolved.
Show resolved Hide resolved
if !module_options.nil?
bittner marked this conversation as resolved.
Show resolved Hide resolved
github_conf = module_options[:github]
gitlab_conf = module_options[:gitlab]
else
github_conf = nil
gitlab_conf = nil
end

if !github_conf.nil?
base_url = github_conf[:base_url] || ENV.fetch('GITHUB_BASE_URL', 'https://api.github.com')
Expand Down
35 changes: 23 additions & 12 deletions lib/modulesync/pr/github.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,25 @@ def manage(namespace, module_name, options)
head = "#{namespace}:#{options[:branch]}"
target_branch = options[:pr_target_branch] || 'master'

pull_requests = @api.pull_requests(repo_path, :state => 'open', :base => target_branch, :head => head)
if pull_requests.empty?
pr = @api.create_pull_request(repo_path,
target_branch,
options[:branch],
options[:pr_title],
options[:message])
if options[:noop]
$stdout.puts \
"Submitted PR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into #{target_branch}"
"Using no-op. Would submit PR '#{options[:pr_title]}' to #{repo_path} " \
"- merges #{options[:branch]} into #{target_branch}"
else
# Skip creating the PR if it exists already.
$stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{options[:branch]}"
pull_requests = @api.pull_requests(repo_path, :state => 'open', :base => target_branch, :head => head)
if pull_requests.empty?
pr = @api.create_pull_request(repo_path,
target_branch,
options[:branch],
options[:pr_title],
options[:message])
$stdout.puts \
"Submitted PR '#{options[:pr_title]}' to #{repo_path} "\
"- merges #{options[:branch]} into #{target_branch}"
else
# Skip creating the PR if it exists already.
$stdout.puts "Skipped! #{pull_requests.length} PRs found for branch #{options[:branch]}"
end
end

# PR labels can either be a list in the YAML file or they can pass in a comma
Expand All @@ -39,8 +46,12 @@ def manage(namespace, module_name, options)
# 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)
if options[:noop]
puts "Using no-op. Would attach the following labels to the PR: #{pr_labels.join(', ')}"
bittner marked this conversation as resolved.
Show resolved Hide resolved
else
$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)
end
end
end
end
Expand Down
38 changes: 23 additions & 15 deletions lib/modulesync/pr/gitlab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,31 @@ def manage(namespace, module_name, options)

head = "#{namespace}:#{options[:branch]}"
target_branch = options[:pr_target_branch] || 'master'
merge_requests = @api.merge_requests(repo_path,
:state => 'opened',
:source_branch => head,
:target_branch => target_branch)
if merge_requests.empty?
mr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
mr = @api.create_merge_request(repo_path, options[:pr_title],
:source_branch => options[:branch],
:target_branch => target_branch,
:labels => mr_labels)

if options[:noop]
$stdout.puts \
"Submitted MR '#{options[:pr_title]}' to #{repo_path} - merges #{options[:branch]} into #{target_branch}"
return if mr_labels.empty?
$stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
"Using no-op. Would submit MR '#{options[:pr_title]}' to #{repo_path} " \
"- merges #{options[:branch]} into #{target_branch}"
else
# Skip creating the MR if it exists already.
$stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{options[:branch]}"
merge_requests = @api.merge_requests(repo_path,
:state => 'opened',
:source_branch => head,
:target_branch => target_branch)
if merge_requests.empty?
mr_labels = ModuleSync::Util.parse_list(options[:pr_labels])
mr = @api.create_merge_request(repo_path, options[:pr_title],
:source_branch => options[:branch],
:target_branch => target_branch,
:labels => mr_labels)
$stdout.puts \
"Submitted MR '#{options[:pr_title]}' to #{repo_path} " \
"- merges #{options[:branch]} into #{target_branch}"
return if mr_labels.empty?
$stdout.puts "Attached the following labels to MR #{mr.iid}: #{mr_labels.join(', ')}"
else
# Skip creating the MR if it exists already.
$stdout.puts "Skipped! #{merge_requests.length} MRs found for branch #{options[:branch]}"
end
end
end
end
Expand Down