diff --git a/.rubocop.yml b/.rubocop.yml index 2e7c69e..d420c8d 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,11 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2016-04-18 08:55:15 -0700 using RuboCop version 0.33.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. + LineLength: Max: 200 @@ -7,6 +15,30 @@ Documentation: StringLiterals: EnforcedStyle: single_quotes +# Offense count: 7 +Metrics/AbcSize: + Max: 25 + +# Offense count: 2 +Metrics/CyclomaticComplexity: + Max: 7 + +# Offense count: 5 +# Configuration parameters: CountComments. +Metrics/MethodLength: + Max: 19 + +# Offense count: 1 +# Configuration parameters: CountComments. +Metrics/ModuleLength: + Max: 123 + +# Offense count: 3 +Style/RescueModifier: + Exclude: + - 'lib/gitx/cli/integrate_command.rb' + - 'lib/gitx/cli/nuke_command.rb' + FileName: Exclude: - bin/* diff --git a/Rakefile b/Rakefile index ede4b71..8a2f2ca 100644 --- a/Rakefile +++ b/Rakefile @@ -3,3 +3,7 @@ require 'bundler/gem_tasks' require 'rspec/core/rake_task' RSpec::Core::RakeTask.new('spec') task default: :spec + +require 'rubocop/rake_task' +RuboCop::RakeTask.new +task default: :rubocop diff --git a/lib/gitx/cli/base_command.rb b/lib/gitx/cli/base_command.rb index 761a48e..b8f8b67 100644 --- a/lib/gitx/cli/base_command.rb +++ b/lib/gitx/cli/base_command.rb @@ -42,11 +42,11 @@ def current_branch end def assert_aggregate_branch!(target_branch) - fail "Invalid aggregate branch: #{target_branch} must be one of supported aggregate branches #{config.aggregate_branches}" unless config.aggregate_branch?(target_branch) + raise "Invalid aggregate branch: #{target_branch} must be one of supported aggregate branches #{config.aggregate_branches}" unless config.aggregate_branch?(target_branch) end def assert_not_protected_branch!(branch, action) - fail "Cannot #{action} reserved branch" if config.reserved_branch?(branch) || config.aggregate_branch?(branch) + raise "Cannot #{action} reserved branch" if config.reserved_branch?(branch) || config.aggregate_branch?(branch) end def config diff --git a/lib/gitx/cli/buildtag_command.rb b/lib/gitx/cli/buildtag_command.rb index 7cfcd61..410fecf 100644 --- a/lib/gitx/cli/buildtag_command.rb +++ b/lib/gitx/cli/buildtag_command.rb @@ -9,7 +9,7 @@ class BuildtagCommand < BaseCommand method_option :branch, type: :string, aliases: '-b', desc: 'branch name for build tag' method_option :message, type: :string, aliases: '-m', desc: 'message to attach to the buildtag' def buildtag - fail "Branch must be one of the supported taggable branches: #{config.taggable_branches}" unless config.taggable_branch?(branch_name) + raise "Branch must be one of the supported taggable branches: #{config.taggable_branches}" unless config.taggable_branch?(branch_name) run_git_cmd 'tag', git_tag, '--annotate', '--message', label run_git_cmd 'push', 'origin', git_tag end diff --git a/lib/gitx/cli/cleanup_command.rb b/lib/gitx/cli/cleanup_command.rb index 47222de..3910362 100644 --- a/lib/gitx/cli/cleanup_command.rb +++ b/lib/gitx/cli/cleanup_command.rb @@ -7,38 +7,47 @@ module Cli class CleanupCommand < BaseCommand desc 'cleanup', 'Cleanup branches that have been merged into master from the repo' def cleanup - checkout_branch config.base_branch - run_git_cmd 'pull' - run_git_cmd 'remote', 'prune', 'origin' - + update_base_branch say 'Deleting local and remote branches that have been merged into ' say config.base_branch, :green - merged_branches(remote: true).each do |branch| + filtered_merged_branches(:remote).each do |branch| run_git_cmd 'push', 'origin', '--delete', branch end - merged_branches(remote: false).each do |branch| + filtered_merged_branches(:local).each do |branch| run_git_cmd 'branch', '--delete', branch end end private + def update_base_branch + checkout_branch config.base_branch + run_git_cmd 'pull' + run_git_cmd 'remote', 'prune', 'origin' + end + # @return list of branches that have been merged - def merged_branches(options = {}) - args = [] - args << '--remote' if options[:remote] - args << '--merged' - output = run_git_cmd('branch', *args).split("\n") - branches = output.map do |branch| - branch = branch.gsub(/\*/, '').strip.split(' ').first - branch = branch.gsub('origin/', '') if options[:remote] - branch + # filter out reserved and aggregate branches + def filtered_merged_branches(source) + merged_branches(source).reject do |branch| + config.reserved_branches.include?(branch) || config.aggregate_branch?(branch) end - branches.uniq! - branches -= config.reserved_branches - branches.reject! { |b| config.aggregate_branch?(b) } + end + + # @return list of branches that have been merged + # see http://stackoverflow.com/questions/26804024/git-branch-merged-sha-via-rugged-libgit2-bindings + def merged_branches(source) + merged_branches = repo.branches.each(source).select do |branch| + target = branch.resolve.target + repo.merge_base(base_branch_merge_target, target) == target.oid + end + merged_branches.map do |branch| + branch.name.gsub('origin/', '') + end + end - branches + def base_branch_merge_target + repo.head.target end end end diff --git a/lib/gitx/cli/nuke_command.rb b/lib/gitx/cli/nuke_command.rb index 2375e05..9331496 100644 --- a/lib/gitx/cli/nuke_command.rb +++ b/lib/gitx/cli/nuke_command.rb @@ -9,7 +9,7 @@ class NukeCommand < BaseCommand method_option :destination, type: :string, aliases: '-d', desc: 'destination branch to reset to' def nuke(bad_branch) good_branch = options[:destination] || ask("What branch do you want to reset #{bad_branch} to? (default: #{bad_branch})") - good_branch = bad_branch if good_branch.length == 0 + good_branch = bad_branch if good_branch.empty? last_known_good_tag = current_build_tag(good_branch) return unless yes?("Reset #{bad_branch} to #{last_known_good_tag}? (y/n)", :green) @@ -47,7 +47,7 @@ def migrations_need_to_be_reverted?(bad_branch, last_known_good_tag) def current_build_tag(branch) last_build_tag = build_tags_for_branch(branch).last - fail "No known good tag found for branch: #{branch}. Verify tag exists via `git tag -l 'build-#{branch}-*'`" unless last_build_tag + raise "No known good tag found for branch: #{branch}. Verify tag exists via `git tag -l 'build-#{branch}-*'`" unless last_build_tag last_build_tag end diff --git a/lib/gitx/cli/review_command.rb b/lib/gitx/cli/review_command.rb index f738130..53724ba 100644 --- a/lib/gitx/cli/review_command.rb +++ b/lib/gitx/cli/review_command.rb @@ -8,14 +8,14 @@ module Cli class ReviewCommand < BaseCommand include Gitx::Github - BUMP_COMMENT_PREFIX = '[gitx] review bump :tada:' + BUMP_COMMENT_PREFIX = '[gitx] review bump :tada:'.freeze BUMP_COMMENT_FOOTER = <<-EOS.dedent # Bump comments should include: # * Summary of what changed # # This footer will automatically be stripped from the created comment EOS - APPROVAL_COMMENT_PREFIX = '[gitx] review approved :shipit:' + APPROVAL_COMMENT_PREFIX = '[gitx] review approved :shipit:'.freeze APPROVAL_COMMENT_FOOTER = <<-EOS.dedent # Approval comments can include: # * Feedback @@ -23,7 +23,7 @@ class ReviewCommand < BaseCommand # # This footer will automatically be stripped from the created comment EOS - REJECTION_COMMENT_PREFIX = '[gitx] review rejected' + REJECTION_COMMENT_PREFIX = '[gitx] review rejected'.freeze REJECTION_COMMENT_FOOTER = <<-EOS.dedent # Rejection comments should include: # * Feedback @@ -42,7 +42,7 @@ class ReviewCommand < BaseCommand method_option :reject, type: :boolean, desc: 'reject the pull request an post comment on pull request' # @see http://developer.github.com/v3/pulls/ def review(branch = nil) - fail 'Github authorization token not found' unless authorization_token + raise 'Github authorization token not found' unless authorization_token branch ||= current_branch.name pull_request = find_or_create_pull_request(branch) diff --git a/lib/gitx/cli/start_command.rb b/lib/gitx/cli/start_command.rb index 0766ecf..bec0bfa 100644 --- a/lib/gitx/cli/start_command.rb +++ b/lib/gitx/cli/start_command.rb @@ -5,7 +5,7 @@ module Gitx module Cli class StartCommand < BaseCommand - EXAMPLE_BRANCH_NAMES = %w( api-fix-invalid-auth desktop-cleanup-avatar-markup share-form-add-edit-link ) + EXAMPLE_BRANCH_NAMES = %w( api-fix-invalid-auth desktop-cleanup-avatar-markup share-form-add-edit-link ).freeze VALID_BRANCH_NAME_REGEX = /^[A-Za-z0-9\-_]+$/ desc 'start', 'start a new git branch with latest changes from master' diff --git a/lib/gitx/configuration.rb b/lib/gitx/configuration.rb index 9cf6bd6..a190705 100644 --- a/lib/gitx/configuration.rb +++ b/lib/gitx/configuration.rb @@ -3,7 +3,7 @@ module Gitx class Configuration - CONFIG_FILE = '.gitx.yml' + CONFIG_FILE = '.gitx.yml'.freeze attr_reader :config diff --git a/lib/gitx/executor.rb b/lib/gitx/executor.rb index 1f0049e..e8429b6 100644 --- a/lib/gitx/executor.rb +++ b/lib/gitx/executor.rb @@ -11,14 +11,16 @@ def execute(*cmd) yield "$ #{cmd.join(' ')}" if block_given? output = '' - Open3.popen2e(*cmd) do |stdin, stdout_err, wait_thread| - while line = stdout_err.gets + Open3.popen2e(*cmd) do |_stdin, stdout_err, wait_thread| + loop do + line = stdout_err.gets + break unless line output << line yield line if block_given? end exit_status = wait_thread.value - fail ExecutionError, "#{cmd.join(' ')} failed" unless exit_status.success? + raise ExecutionError, "#{cmd.join(' ')} failed" unless exit_status.success? end output end diff --git a/lib/gitx/extensions/string.rb b/lib/gitx/extensions/string.rb index 5a41d02..b32078d 100644 --- a/lib/gitx/extensions/string.rb +++ b/lib/gitx/extensions/string.rb @@ -4,7 +4,7 @@ def undent indent = scan(/^[ \t]*(?=\S)/).min.size || 0 gsub(/^[ \t]{#{indent}}/, '') end - alias_method :dedent, :undent + alias dedent undent def blank? to_s == '' diff --git a/lib/gitx/extensions/thor.rb b/lib/gitx/extensions/thor.rb index b48ea00..56fdcc7 100644 --- a/lib/gitx/extensions/thor.rb +++ b/lib/gitx/extensions/thor.rb @@ -12,13 +12,13 @@ def ask_editor(initial_text = '', editor: nil, footer: nil) f.flush flags = case editor - when 'mate', 'emacs', 'subl' - '-w' - when 'mvim' - '-f' - else - '' - end + when 'mate', 'emacs', 'subl' + '-w' + when 'mvim' + '-f' + else + '' + end pid = fork { exec([editor, flags, f.path].join(' ')) } Process.waitpid(pid) File.read(f.path) diff --git a/lib/gitx/github.rb b/lib/gitx/github.rb index 152f491..0437e54 100644 --- a/lib/gitx/github.rb +++ b/lib/gitx/github.rb @@ -4,9 +4,9 @@ module Gitx module Github - GLOBAL_CONFIG_FILE = '~/.config/gitx/github.yml' - REVIEW_CONTEXT = 'peer_review' - CLIENT_URL = 'https://github.com/wireframe/gitx' + GLOBAL_CONFIG_FILE = '~/.config/gitx/github.yml'.freeze + REVIEW_CONTEXT = 'peer_review'.freeze + CLIENT_URL = 'https://github.com/wireframe/gitx'.freeze PULL_REQUEST_FOOTER = <<-EOS.dedent # Pull Request Protips(tm): # * Describe how this change accomplishes the task at hand @@ -72,14 +72,14 @@ def create_pull_request(branch) end def pull_request_body(branch) - changelog = run_git_cmd('log', "origin/#{config.base_branch}...#{branch}", '--reverse', '--no-merges', "--pretty=format:* %B") + changelog = run_git_cmd('log', "origin/#{config.base_branch}...#{branch}", '--reverse', '--no-merges', '--pretty=format:* %B') description = options[:description] description_template = [] description_template << "#{description}\n" if description description_template << changelog - body = ask_editor(description_template.join("\n"), editor: repo.config['core.editor'], footer: PULL_REQUEST_FOOTER) + ask_editor(description_template.join("\n"), editor: repo.config['core.editor'], footer: PULL_REQUEST_FOOTER) end def pull_request_title(branch) @@ -120,7 +120,7 @@ def create_authorization def github_client_name timestamp = Time.now.utc.strftime('%FT%R:%S%z') - client_name = "Git eXtensions #{timestamp}" + "Git eXtensions #{timestamp}" end def github_client @@ -131,7 +131,7 @@ def github_client # @raise error if github.user is not configured def username username = repo.config['github.user'] - fail "Github user not configured. Run: `git config --global github.user 'me@email.com'`" unless username + raise "Github user not configured. Run: `git config --global github.user 'me@email.com'`" unless username username end @@ -142,7 +142,7 @@ def username # https://github.com/wireframe/gitx.git #=> wireframe/gitx def github_slug remote = repo.config['remote.origin.url'] - remote.to_s.gsub(/\.git$/, '').split(/[:\/]/).last(2).join('/') + remote.to_s.gsub(/\.git$/, '').split(%r{[:\/]}).last(2).join('/') end def github_organization diff --git a/lib/gitx/version.rb b/lib/gitx/version.rb index 6fb5da4..d00c4a8 100644 --- a/lib/gitx/version.rb +++ b/lib/gitx/version.rb @@ -1,3 +1,3 @@ module Gitx - VERSION = '2.23.0' + VERSION = '2.23.1'.freeze end diff --git a/spec/gitx/cli/cleanup_command_spec.rb b/spec/gitx/cli/cleanup_command_spec.rb index ad217e8..8279983 100644 --- a/spec/gitx/cli/cleanup_command_spec.rb +++ b/spec/gitx/cli/cleanup_command_spec.rb @@ -11,20 +11,29 @@ end let(:cli) { described_class.new(args, options, config) } let(:executor) { cli.send(:executor) } - let(:branch) { double('fake branch', name: 'feature-branch') } - let(:remote_branches) { '' } - let(:local_branches) { '' } + let(:remote_branches) { [] } + let(:local_branches) { [] } + let(:workdir) { '.' } + let(:oid) { '123123' } + let(:target) { double(:target, oid: oid) } + let(:reference) { double(:ref, target: target) } + let(:repo) { double(:repo, workdir: workdir, head: reference) } + let(:branches) { double(:branches) } before do - allow(cli).to receive(:current_branch).and_return(branch) + allow(cli).to receive(:repo).and_return(repo) + allow(repo).to receive(:branches).and_return(branches) + allow(repo).to receive(:merge_base).with(target, target).and_return(oid) + allow(branches).to receive(:each).with(:local).and_return(local_branches) + allow(branches).to receive(:each).with(:remote).and_return(remote_branches) end describe '#cleanup' do context 'when merged local branches exist' do let(:local_branches) do - <<-EOS.dedent - merged-local-feature - EOS + [ + double(:branch, name: 'merged-local-feature', resolve: reference) + ] end before do allow(cli).to receive(:say) @@ -32,8 +41,6 @@ expect(executor).to receive(:execute).with('git', 'checkout', 'master').ordered expect(executor).to receive(:execute).with('git', 'pull').ordered expect(executor).to receive(:execute).with('git', 'remote', 'prune', 'origin').ordered - expect(executor).to receive(:execute).with('git', 'branch', '--remote', '--merged').and_return(remote_branches).ordered - expect(executor).to receive(:execute).with('git', 'branch', '--merged').and_return(local_branches).ordered expect(executor).to receive(:execute).with('git', 'branch', '--delete', 'merged-local-feature').ordered cli.cleanup @@ -44,9 +51,9 @@ end context 'when merged remote branches exist' do let(:remote_branches) do - <<-EOS.dedent - origin/merged-remote-feature - EOS + [ + double(:branch, name: 'origin/merged-remote-feature', resolve: reference) + ] end before do allow(cli).to receive(:say) @@ -54,9 +61,7 @@ expect(executor).to receive(:execute).with('git', 'checkout', 'master').ordered expect(executor).to receive(:execute).with('git', 'pull').ordered expect(executor).to receive(:execute).with('git', 'remote', 'prune', 'origin').ordered - expect(executor).to receive(:execute).with('git', 'branch', '--remote', '--merged').and_return(remote_branches).ordered expect(executor).to receive(:execute).with('git', 'push', 'origin', '--delete', 'merged-remote-feature').ordered - expect(executor).to receive(:execute).with('git', 'branch', '--merged').and_return(local_branches).ordered cli.cleanup end @@ -66,9 +71,9 @@ end context 'when mreged remote branches with slash exist' do let(:remote_branches) do - <<-EOS.dedent - origin/merged-remote-feature/review - EOS + [ + double(:branch, name: 'origin/merged-remote-feature/review', resolve: reference) + ] end before do allow(cli).to receive(:say) @@ -76,9 +81,7 @@ expect(executor).to receive(:execute).with('git', 'checkout', 'master').ordered expect(executor).to receive(:execute).with('git', 'pull').ordered expect(executor).to receive(:execute).with('git', 'remote', 'prune', 'origin').ordered - expect(executor).to receive(:execute).with('git', 'branch', '--remote', '--merged').and_return(remote_branches).ordered expect(executor).to receive(:execute).with('git', 'push', 'origin', '--delete', 'merged-remote-feature/review').ordered - expect(executor).to receive(:execute).with('git', 'branch', '--merged').and_return(local_branches).ordered cli.cleanup end diff --git a/spec/gitx/cli/integrate_command_spec.rb b/spec/gitx/cli/integrate_command_spec.rb index ab50a5c..cf72bbe 100644 --- a/spec/gitx/cli/integrate_command_spec.rb +++ b/spec/gitx/cli/integrate_command_spec.rb @@ -84,7 +84,7 @@ expect(executor).to receive(:execute).with('git', 'update').ordered expect(executor).to receive(:execute).with('git', 'checkout', 'feature-branch').ordered expect(executor).to receive(:execute).with('git', 'update').ordered - expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', "--pretty=format:* %B").and_return(changelog).ordered + expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', '--pretty=format:* %B').and_return(changelog).ordered expect(executor).to receive(:execute).with('git', 'fetch', 'origin').ordered expect(executor).to receive(:execute).with('git', 'branch', '--delete', '--force', 'staging').ordered expect(executor).to receive(:execute).with('git', 'checkout', 'staging').ordered @@ -158,7 +158,8 @@ expect(executor).to receive(:execute).with('git', 'fetch', 'origin').ordered expect(executor).to receive(:execute).with('git', 'branch', '--delete', '--force', 'staging').ordered expect(executor).to receive(:execute).with('git', 'checkout', 'staging').ordered - expect(executor).to receive(:execute).with('git', 'merge', '--no-ff', '--message', '[gitx] Integrating feature-branch into staging (Pull request #10)', 'feature-branch').and_raise('git merge feature-branch failed').ordered + expect(executor).to receive(:execute).with('git', 'merge', '--no-ff', '--message', '[gitx] Integrating feature-branch into staging (Pull request #10)', 'feature-branch') + .and_raise('git merge feature-branch failed').ordered VCR.use_cassette('pull_request_does_exist_with_success_status') do expect { cli.integrate }.to raise_error(/Merge conflict occurred. Please fix merge conflict and rerun command with --resume feature-branch flag/) diff --git a/spec/gitx/cli/release_command_spec.rb b/spec/gitx/cli/release_command_spec.rb index 01427b3..5ac94f7 100644 --- a/spec/gitx/cli/release_command_spec.rb +++ b/spec/gitx/cli/release_command_spec.rb @@ -152,7 +152,7 @@ expect(executor).to receive(:execute).with('git', 'update').ordered expect(executor).to receive(:execute).with('git', 'checkout', 'feature-branch').ordered expect(executor).to receive(:execute).with('git', 'update').ordered - expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', "--pretty=format:* %B").and_return('2013-01-01 did some stuff').ordered + expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', '--pretty=format:* %B').and_return('2013-01-01 did some stuff').ordered expect(executor).to receive(:execute).with('git', 'checkout', 'master').ordered expect(executor).to receive(:execute).with('git', 'pull', 'origin', 'master').ordered expect(executor).to receive(:execute).with('git', 'merge', '--no-ff', '--message', '[gitx] Releasing feature-branch to master (Pull request #10)', 'feature-branch').ordered diff --git a/spec/gitx/cli/review_command_spec.rb b/spec/gitx/cli/review_command_spec.rb index b3e2aa9..05e36d3 100644 --- a/spec/gitx/cli/review_command_spec.rb +++ b/spec/gitx/cli/review_command_spec.rb @@ -46,7 +46,7 @@ allow(cli).to receive(:authorization_token).and_return(authorization_token) expect(executor).to receive(:execute).with('git', 'checkout', 'feature-branch').ordered expect(executor).to receive(:execute).with('git', 'update').ordered - expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', "--pretty=format:* %B").and_return(changelog).ordered + expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', '--pretty=format:* %B').and_return(changelog).ordered expect(cli).to receive(:ask_editor).with(changelog, hash_including(footer: Gitx::Github::PULL_REQUEST_FOOTER)).and_return('description') stub_request(:post, 'https://api.github.com/repos/wireframe/gitx/pulls').to_return(status: 201, body: new_pull_request.to_json, headers: { 'Content-Type' => 'application/json' }) @@ -82,11 +82,11 @@ allow(cli).to receive(:authorization_token).and_return(authorization_token) expect(executor).to receive(:execute).with('git', 'checkout', 'feature-branch').ordered expect(executor).to receive(:execute).with('git', 'update').ordered - expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', "--pretty=format:* %B").and_return(changelog).ordered + expect(executor).to receive(:execute).with('git', 'log', 'origin/master...feature-branch', '--reverse', '--no-merges', '--pretty=format:* %B').and_return(changelog).ordered expect(cli).to receive(:ask_editor).with(changelog, hash_including(footer: Gitx::Github::PULL_REQUEST_FOOTER)).and_return(pull_request_description) stub_request(:post, 'https://api.github.com/repos/wireframe/gitx/pulls') - .with(body: {base: 'master', head: 'feature-branch', title: 'feature branch', body: pull_request_description}.to_json) + .with(body: { base: 'master', head: 'feature-branch', title: 'feature branch', body: pull_request_description }.to_json) .to_return(status: 201, body: new_pull_request.to_json, headers: { 'Content-Type' => 'application/json' }) VCR.use_cassette('pull_request_does_not_exist') do diff --git a/spec/support/stub_execution.rb b/spec/support/stub_execution.rb index 88e705a..cfa5743 100644 --- a/spec/support/stub_execution.rb +++ b/spec/support/stub_execution.rb @@ -9,6 +9,6 @@ def execute_with_stub(cmd) end end - alias_method :execute_without_stub, :` - alias_method :`, :execute_with_stub + alias execute_without_stub ` + alias ` execute_with_stub end