remove more needless bang methods

app/controllers/api/automated_deploys_controller.rb

@@ -10,7 +10,7 @@ def create
     env = params.require(:env).to_unsafe_hash.map { |k, v| "export PARAM_#{k}=#{v.gsub("\n", "\\n").shellescape}" }
     env << "export DEPLOY_GROUPS=#{@deploy_group.env_value}"
 
-    deploy = deploy_service.deploy!(
+    deploy = deploy_service.deploy(
       @stage,
       reference: @last_deploy.reference,
       buddy_id: @last_deploy.buddy_id || @last_deploy.job.user_id,

app/controllers/builds_controller.rb

@@ -65,7 +65,7 @@ def edit_build_params
   end
 
   def start_docker_build
-    DockerBuilderService.new(@build).run!(push: true)
+    DockerBuilderService.new(@build).run(push: true)
   end
 
   def new_or_modified_build

app/controllers/deploy_groups_controller.rb

@@ -75,7 +75,7 @@ def deploy_all
       next unless last_success_deploy
 
       deploy_service = DeployService.new(current_user)
-      deploy_service.deploy!(stage, reference: last_success_deploy.reference)
+      deploy_service.deploy(stage, reference: last_success_deploy.reference)
     end.compact
 
     if deploys.empty?

app/controllers/deploys_controller.rb

@@ -50,7 +50,7 @@ def new
 
   def create
     deploy_service = DeployService.new(current_user)
-    @deploy = deploy_service.deploy!(stage, deploy_params)
+    @deploy = deploy_service.deploy(stage, deploy_params)
 
     respond_to do |format|
       format.html do

app/controllers/integrations/base_controller.rb

@@ -59,14 +59,14 @@ def release_params
   def find_or_create_release
     latest_release = project.releases.order(:id).last
     return latest_release if latest_release&.contains_commit?(commit)
-    ReleaseService.new(project).release!(release_params)
+    ReleaseService.new(project).release(release_params)
   end
 
   # returns stage that failed to deploy or nil
   def deploy_to_stages(release, stages)
     deploy_service = DeployService.new(user)
     stages.detect do |stage|
-      deploy = deploy_service.deploy!(stage, reference: release&.version || commit)
+      deploy = deploy_service.deploy(stage, reference: release&.version || commit)
       if deploy.new_record?
         record_log :error, "Deploy to #{stage.name} failed: #{deploy.errors.full_messages}"
         true
@@ -114,7 +114,7 @@ def service_name
   def create_docker_image(release)
     build = find_or_create_build(branch)
     release.update_attribute(:build, build) if release
-    DockerBuilderService.new(build).run!(push: true, tag_as_latest: true)
+    DockerBuilderService.new(build).run(push: true, tag_as_latest: true)
   end
 
   # TODO: use first_or_create here ... some weird rails bug breaks

app/controllers/releases_controller.rb

@@ -21,7 +21,7 @@ def new
   end
 
   def create
-    @release = ReleaseService.new(@project).release!(release_params)
+    @release = ReleaseService.new(@project).release(release_params)
     if @release.persisted?
       redirect_to [@project, @release]
     else

app/models/deploy.rb

@@ -110,7 +110,7 @@ def waiting_for_buddy?
 
   def confirm_buddy!(buddy)
     update_attributes!(buddy: buddy, started_at: Time.now)
-    start!
+    start
   end
 
   def start_time
@@ -124,7 +124,7 @@ def duration
   def self.start_deploys_waiting_for_restart!
     pending.reject(&:waiting_for_buddy?).each do |deploy|
       deploy.touch # HACK: refresh is immediate with update
-      deploy.send :start!
+      deploy.send :start
     end
   end
 
@@ -203,8 +203,8 @@ def csv_line
 
   private
 
-  def start!
-    DeployService.new(user).confirm_deploy!(self)
+  def start
+    DeployService.new(user).confirm_deploy(self)
   end
 
   def summary_action

app/models/deploy_service.rb

@@ -7,7 +7,7 @@ def initialize(user)
     @user = user
   end
 
-  def deploy!(stage, attributes)
+  def deploy(stage, attributes)
     deploy = stage.create_deploy(user, attributes)
 
     if deploy.persisted?
@@ -22,14 +22,14 @@ def deploy!(stage, attributes)
       if deploy.waiting_for_buddy? && !copy_approval_from_last_deploy(deploy)
         Samson::Hooks.fire(:buddy_request, deploy)
       else
-        confirm_deploy!(deploy)
+        confirm_deploy(deploy)
       end
     end
 
     deploy
   end
 
-  def confirm_deploy!(deploy)
+  def confirm_deploy(deploy)
     stage = deploy.stage
 
     job_execution = JobExecution.new(deploy.reference, deploy.job, env: construct_env(stage))

app/models/docker_builder_service.rb

@@ -16,7 +16,7 @@ def build_docker_image(dir, output, dockerfile: nil, tag: nil)
         file = " -f #{dockerfile.shellescape}" if dockerfile
         build = "docker build#{file}#{tag} ."
         executor = TerminalExecutor.new(output)
-        return unless executor.execute!(
+        return unless executor.execute(
           "cd #{dir.shellescape}",
           *login_commands,
           executor.verbose_command(build)
@@ -66,7 +66,7 @@ def initialize(build)
     @build = build
   end
 
-  def run!(push: false, tag_as_latest: false)
+  def run(push: false, tag_as_latest: false)
     return unless Rails.cache.write("build-service-#{build.id}", true, unless_exist: true, expires_in: 10.seconds)
     build.docker_build_job&.destroy # if there's an old build job, delete it
     build.docker_tag = build.label.try(:parameterize).presence || 'latest'
@@ -112,7 +112,7 @@ def run_build_image_job(local_job, push: false, tag_as_latest: false)
       job: local_job,
       registry: DockerRegistry.first
     )
-    success, build_log = k8s_job.execute!(
+    success, build_log = k8s_job.execute(
       build, project,
       docker_tag: build.docker_tag,
       push: push,
@@ -138,7 +138,7 @@ def run_build_image_job(local_job, push: false, tag_as_latest: false)
   def execute_build_command(tmp_dir, command)
     return unless command
     commands = execution.base_commands(tmp_dir) + command.command.split(/\r?\n|\r/)
-    unless execution.executor.execute!(*commands)
+    unless execution.executor.execute(*commands)
       raise Samson::Hooks::UserError, "Error running build command"
     end
   end

app/models/git_repository.rb

@@ -121,17 +121,17 @@ def outside_caller
   end
 
   def clone!
-    executor.execute! "git -c core.askpass=true clone --mirror #{repository_url} #{repo_cache_dir}"
+    executor.execute "git -c core.askpass=true clone --mirror #{repository_url} #{repo_cache_dir}"
   end
   add_method_tracer :clone!
 
   def create_workspace(temp_dir)
-    executor.execute! "git clone #{repo_cache_dir} #{temp_dir}"
+    executor.execute "git clone #{repo_cache_dir} #{temp_dir}"
   end
   add_method_tracer :create_workspace
 
   def update!
-    executor.execute!("cd #{repo_cache_dir}", 'git fetch -p')
+    executor.execute("cd #{repo_cache_dir}", 'git fetch -p')
   end
   add_method_tracer :update!
 
@@ -140,14 +140,14 @@ def sha_exist?(sha)
   end
 
   def checkout(git_reference, pwd)
-    executor.execute!("cd #{pwd}", "git checkout --quiet #{git_reference.shellescape}")
+    executor.execute("cd #{pwd}", "git checkout --quiet #{git_reference.shellescape}")
   end
 
   def checkout_submodules(pwd)
     return true unless File.exist? "#{pwd}/.gitmodules"
 
     recursive_flag = " --recursive" if git_supports_recursive_flag?
-    executor.execute!(
+    executor.execute(
       "cd #{pwd}",
       "git submodule sync#{recursive_flag}",
       "git submodule update --init --recursive"

app/models/job_execution.rb

@@ -43,11 +43,11 @@ def initialize(reference, job, env: {}, output: OutputBuffer.new, &block)
     end
   end
 
-  def start!
-    @thread = Thread.new { ActiveRecord::Base.connection_pool.with_connection { run! } }
+  def start
+    @thread = Thread.new { ActiveRecord::Base.connection_pool.with_connection { run } }
   end
 
-  def wait!
+  def wait
     @thread.join
   end
 
@@ -111,18 +111,18 @@ def error!(exception)
     @job.errored! if @job.active?
   end
 
-  def run!
+  def run
     @output.write('', :started)
     @start_callbacks.each(&:call)
     @job.running!
 
     success = make_tempdir do |dir|
-      return @job.errored! unless setup!(dir)
+      return @job.errored! unless setup(dir)
 
       if @execution_block
         @execution_block.call(self, dir)
       else
-        execute!(dir)
+        execute(dir)
       end
     end
 
@@ -138,7 +138,7 @@ def run!
   ensure
     finish unless @cancelled
   end
-  add_transaction_tracer :run!,
+  add_transaction_tracer :run,
     category: :task,
     params: '{ job_id: id, project: job.project.try(:name), reference: reference }'
 
@@ -148,7 +148,7 @@ def finish
     @finish_callbacks.each(&:call)
   end
 
-  def execute!(dir)
+  def execute(dir)
     Samson::Hooks.fire(:after_deploy_setup, dir, @job, @output, @reference)
 
     @output.write("\n# Executing deploy\n")
@@ -165,9 +165,9 @@ def execute!(dir)
       payload[:success] =
         if defined?(Kubernetes::DeployExecutor) && stage&.kubernetes
           @executor = Kubernetes::DeployExecutor.new(@output, job: @job, reference: @reference)
-          @executor.execute!
+          @executor.execute
         else
-          @executor.execute!(*cmds)
+          @executor.execute(*cmds)
         end
     end
 
@@ -176,7 +176,7 @@ def execute!(dir)
     payload[:success]
   end
 
-  def setup!(dir)
+  def setup(dir)
     return unless resolve_ref_to_commit
     stage.try(:kubernetes) || @repository.checkout_workspace(dir, @reference)
   end

app/models/job_queue.rb

@@ -65,7 +65,7 @@ def debug
 
   def start_job(job_execution, queue)
     @active[queue] = job_execution
-    job_execution.start!
+    job_execution.start
   end
 
   def delete_and_enqueue_next(queue_name, job_execution)

app/models/release_service.rb

@@ -4,7 +4,7 @@ def initialize(project)
     @project = project
   end
 
-  def release!(attrs = {})
+  def release(attrs = {})
     release = @project.releases.create(attrs)
     if release.persisted?
       push_tag_to_git_repository(release)
@@ -30,7 +30,7 @@ def start_deploys(release)
 
     @project.stages.deployed_on_release.each do |stage|
       if Samson::Hooks.fire(:release_deploy_conditions, stage, release).all?
-        deploy_service.deploy!(stage, reference: release.version)
+        deploy_service.deploy(stage, reference: release.version)
       end
     end
   end

app/models/terminal_executor.rb

@@ -25,8 +25,7 @@ def initialize(output, verbose: false, deploy: nil, project: nil)
     @cancelled = false
   end
 
-  # TODO: rename to execute since it does not blow up on failure
-  def execute!(*commands)
+  def execute(*commands)
     return false if @cancelled
     options = {in: '/dev/null', unsetenv_others: true}
     output, input, pid = PTY.spawn(whitelisted_env, script(commands), options)

app/models/terminal_output_scanner.rb

@@ -9,7 +9,7 @@ class TerminalOutputScanner
   def initialize(source)
     @source = source
     @queue = []
-    reset_buffer!
+    reset_buffer
   end
 
   def each(&block)
@@ -37,7 +37,7 @@ def write(data)
 
   def write_part(part)
     if part.start_with?("\r", "\n")
-      flush_buffer!
+      flush_buffer
 
       part.sub!(/^\r/, "") # chop off the leading \r
 
@@ -51,17 +51,17 @@ def write_part(part)
 
     @buffer << part
 
-    flush_buffer! if @buffer.end_with?("\n")
+    flush_buffer if @buffer.end_with?("\n")
   end
 
-  def flush_buffer!
+  def flush_buffer
     unless @buffer.empty?
       output(@state, @buffer)
-      reset_buffer!
+      reset_buffer
     end
   end
 
-  def reset_buffer!
+  def reset_buffer
     @buffer = "".dup
     @state = :append
   end

lib/samson/periodical_deploy.rb

@@ -13,7 +13,7 @@ def self.run
         next unless deploy.succeeded?
 
         deployer = User.where(external_id: EXTERNAL_ID).first!
-        DeployService.new(deployer).deploy!(
+        DeployService.new(deployer).deploy(
           stage,
           reference: deploy.reference,
           buddy_id: deploy.buddy_id || deploy.job.user_id

plugins/docker_binary_builder/app/models/binary_builder.rb

@@ -44,7 +44,7 @@ def run_pre_build_script
     return unless File.file? pre_build_file
 
     @output.puts "Running pre build script..."
-    unless @executor.execute! pre_build_file
+    unless @executor.execute pre_build_file
       raise Samson::Hooks::UserError, "Error running pre build script"
     end
   end