From 507fa863d5ae9dfae15a85d989f34acba8d653fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:02:21 +0200 Subject: [PATCH 01/16] SImplify outdated command I don't know why a nil group was being artificially added and then always skipped. --- lib/bundler/cli/outdated.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 0b710e97822..3645db34f43 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -110,13 +110,9 @@ def run if options_include_groups ordered_groups = outdated_gems_by_groups.keys.compact.sort - ordered_groups.insert(0, nil).each do |groups| + ordered_groups.each do |groups| gems = outdated_gems_by_groups[groups] - contains_group = if groups - groups.split(", ").include?(options[:group]) - else - options[:group] == "group" - end + contains_group = groups.split(", ").include?(options[:group]) next if (!options[:groups] && !contains_group) || gems.nil? From 38eab3a3768966bafa71ddaf18378aeba3a53ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:09:36 +0200 Subject: [PATCH 02/16] Little optimization to `bundle outdated` No need to check the target group if the group has no outdated gems. --- lib/bundler/cli/outdated.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 3645db34f43..b7d9b77d5ed 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -112,9 +112,10 @@ def run ordered_groups = outdated_gems_by_groups.keys.compact.sort ordered_groups.each do |groups| gems = outdated_gems_by_groups[groups] + next if gems.nil? contains_group = groups.split(", ").include?(options[:group]) - next if (!options[:groups] && !contains_group) || gems.nil? + next if !options[:groups] && !contains_group unless options[:parseable] Bundler.ui.info(header_group_message(groups)) From 1b70ddba2e9607f4f25ac2e227a15e3cd5d84a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:11:05 +0200 Subject: [PATCH 03/16] `unless` is much more readable than `if` here --- lib/bundler/cli/outdated.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index b7d9b77d5ed..d3c68bb45ae 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -115,7 +115,7 @@ def run next if gems.nil? contains_group = groups.split(", ").include?(options[:group]) - next if !options[:groups] && !contains_group + next unless options[:groups] || contains_group unless options[:parseable] Bundler.ui.info(header_group_message(groups)) From 2a3ddf479606a3595f424baecfc191cd72e0e3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:11:50 +0200 Subject: [PATCH 04/16] Little reformat Move each `next` to the variable it uses for its condition. --- lib/bundler/cli/outdated.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index d3c68bb45ae..028bb3b05ea 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -113,8 +113,8 @@ def run ordered_groups.each do |groups| gems = outdated_gems_by_groups[groups] next if gems.nil? - contains_group = groups.split(", ").include?(options[:group]) + contains_group = groups.split(", ").include?(options[:group]) next unless options[:groups] || contains_group unless options[:parseable] From 00472b1fe8b54630f2d3a86080337c46e256fecd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:12:59 +0200 Subject: [PATCH 05/16] Spare a local variable --- lib/bundler/cli/outdated.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 028bb3b05ea..ba654baf373 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -109,8 +109,7 @@ def run end if options_include_groups - ordered_groups = outdated_gems_by_groups.keys.compact.sort - ordered_groups.each do |groups| + outdated_gems_by_groups.keys.compact.sort.each do |groups| gems = outdated_gems_by_groups[groups] next if gems.nil? From 61fd42db9ac5f598ac784c8816d8404148ace8a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:16:43 +0200 Subject: [PATCH 06/16] There should be no `nil` keys here --- lib/bundler/cli/outdated.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index ba654baf373..b15e7b61b91 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -109,7 +109,7 @@ def run end if options_include_groups - outdated_gems_by_groups.keys.compact.sort.each do |groups| + outdated_gems_by_groups.keys.sort.each do |groups| gems = outdated_gems_by_groups[groups] next if gems.nil? From 8bf7a6d80865d6ecfba3cdced140b5a49a6dbb95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:21:51 +0200 Subject: [PATCH 07/16] Traverse `outdated_gems_by_groups` in a simpler way --- lib/bundler/cli/outdated.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index b15e7b61b91..1bcaf3f0d42 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -109,8 +109,7 @@ def run end if options_include_groups - outdated_gems_by_groups.keys.sort.each do |groups| - gems = outdated_gems_by_groups[groups] + outdated_gems_by_groups.sort.each do |groups, gems| next if gems.nil? contains_group = groups.split(", ").include?(options[:group]) From 0ad599d7c66175004ed3d02187aa4b93295b0184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:26:58 +0200 Subject: [PATCH 08/16] Invert another `if` for readability --- lib/bundler/cli/outdated.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 1bcaf3f0d42..667b8ed4c6b 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -76,7 +76,7 @@ def run end specs.sort_by(&:name).each do |current_spec| - next if !gems.empty? && !gems.include?(current_spec.name) + next unless gems.empty? || gems.include?(current_spec.name) dependency = current_dependencies[current_spec.name] active_spec = retrieve_active_spec(definition, current_spec) From a707261552aae0f9a32edfb30f7bb32ce64e5d81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:27:14 +0200 Subject: [PATCH 09/16] Move local variable to where it's used --- lib/bundler/cli/outdated.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 667b8ed4c6b..ddc9271b957 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -78,7 +78,6 @@ def run specs.sort_by(&:name).each do |current_spec| next unless gems.empty? || gems.include?(current_spec.name) - dependency = current_dependencies[current_spec.name] active_spec = retrieve_active_spec(definition, current_spec) next if active_spec.nil? @@ -87,6 +86,8 @@ def run gem_outdated = Gem::Version.new(active_spec.version) > Gem::Version.new(current_spec.version) next unless gem_outdated || (current_spec.git_version != active_spec.git_version) + + dependency = current_dependencies[current_spec.name] groups = nil if dependency && !options[:parseable] groups = dependency.groups.join(", ") From 0e748fc10f4a284392dfdba1f18474afc63c88ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:27:46 +0200 Subject: [PATCH 10/16] Unify to a single `next` call --- lib/bundler/cli/outdated.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index ddc9271b957..e75a4c71be7 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -79,8 +79,6 @@ def run next unless gems.empty? || gems.include?(current_spec.name) active_spec = retrieve_active_spec(definition, current_spec) - - next if active_spec.nil? next if filter_options_patch.any? && !update_present_via_semver_portions(current_spec, active_spec, options) @@ -235,6 +233,8 @@ def check_for_deployment_mode! end def update_present_via_semver_portions(current_spec, active_spec, options) + return false if active_spec.nil? + current_major = current_spec.version.segments.first active_major = active_spec.version.segments.first From 0b8958164cf0ce07a8e5d691a409f6eeda18ba00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:28:19 +0200 Subject: [PATCH 11/16] Merge condition to a single line I think this kind of artificial line breaks are distracting. --- lib/bundler/cli/outdated.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index e75a4c71be7..25c0dc3ee5f 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -79,8 +79,7 @@ def run next unless gems.empty? || gems.include?(current_spec.name) active_spec = retrieve_active_spec(definition, current_spec) - next if filter_options_patch.any? && - !update_present_via_semver_portions(current_spec, active_spec, options) + next if filter_options_patch.any? && !update_present_via_semver_portions(current_spec, active_spec, options) gem_outdated = Gem::Version.new(active_spec.version) > Gem::Version.new(current_spec.version) next unless gem_outdated || (current_spec.git_version != active_spec.git_version) From 3d8c1e793de5131beeb199913646b0858739feeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 11:28:54 +0200 Subject: [PATCH 12/16] Invert yet another `if` to `unless` for readability --- lib/bundler/cli/outdated.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 25c0dc3ee5f..eb7de745c1d 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -79,7 +79,7 @@ def run next unless gems.empty? || gems.include?(current_spec.name) active_spec = retrieve_active_spec(definition, current_spec) - next if filter_options_patch.any? && !update_present_via_semver_portions(current_spec, active_spec, options) + next unless filter_options_patch.empty? || update_present_via_semver_portions(current_spec, active_spec, options) gem_outdated = Gem::Version.new(active_spec.version) > Gem::Version.new(current_spec.version) next unless gem_outdated || (current_spec.git_version != active_spec.git_version) From 68a7cee2c2c7907a4b7402551f73ecd65787c2d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 15:11:35 +0200 Subject: [PATCH 13/16] Remove unnecessary `next` Groups are only added to this hash if they have gems. --- lib/bundler/cli/outdated.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index eb7de745c1d..dd890841bce 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -108,8 +108,6 @@ def run if options_include_groups outdated_gems_by_groups.sort.each do |groups, gems| - next if gems.nil? - contains_group = groups.split(", ").include?(options[:group]) next unless options[:groups] || contains_group From 9650085700a6a7fa22856e21c678dc891b1cd380 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 13:13:08 +0200 Subject: [PATCH 14/16] Remove some artificial line breaks --- lib/bundler/cli/outdated.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index dd890841bce..5c0da983250 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -10,8 +10,7 @@ def initialize(options, gems) @gems = gems @sources = Array(options[:source]) - @filter_options_patch = options.keys & - %w[filter-major filter-minor filter-patch] + @filter_options_patch = options.keys & %w[filter-major filter-minor filter-patch] @outdated_gems_by_groups = {} @outdated_gems_list = [] @@ -22,8 +21,7 @@ def initialize(options, gems) # the patch level options imply strict is also true. It wouldn't make # sense otherwise. - @strict = options["filter-strict"] || - Bundler::CLI::Common.patch_level_options(options).any? + @strict = options["filter-strict"] || Bundler::CLI::Common.patch_level_options(options).any? end def run From 041271a0ae573584a98dc7a1b52a7cab8eadb71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 16:14:25 +0200 Subject: [PATCH 15/16] Indentation fixes --- spec/commands/outdated_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/commands/outdated_spec.rb b/spec/commands/outdated_spec.rb index ab549257564..855f64056aa 100644 --- a/spec/commands/outdated_spec.rb +++ b/spec/commands/outdated_spec.rb @@ -658,19 +658,19 @@ def test_group_option(group = nil, gems_list_size = 1) # establish a lockfile set to 1.0.0 install_gemfile <<-G - source "#{file_uri_for(gem_repo4)}" - gem 'patch', '1.0.0' - gem 'minor', '1.0.0' - gem 'major', '1.0.0' + source "#{file_uri_for(gem_repo4)}" + gem 'patch', '1.0.0' + gem 'minor', '1.0.0' + gem 'major', '1.0.0' G # remove 1.4.3 requirement and bar altogether # to setup update specs below gemfile <<-G - source "#{file_uri_for(gem_repo4)}" - gem 'patch' - gem 'minor' - gem 'major' + source "#{file_uri_for(gem_repo4)}" + gem 'patch' + gem 'minor' + gem 'major' G end From b7bc8d4b22273aa45c08f4eb111e3f5c36d78065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Tue, 24 Sep 2019 16:22:05 +0200 Subject: [PATCH 16/16] Remove unnecessary `outdated_gems_by_groups` --- lib/bundler/cli/outdated.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb index 5c0da983250..2a2df4c26e9 100644 --- a/lib/bundler/cli/outdated.rb +++ b/lib/bundler/cli/outdated.rb @@ -3,7 +3,7 @@ module Bundler class CLI::Outdated attr_reader :options, :gems, :options_include_groups, :filter_options_patch, :sources, :strict - attr_accessor :outdated_gems_by_groups, :outdated_gems_list + attr_accessor :outdated_gems_list def initialize(options, gems) @options = options @@ -12,7 +12,6 @@ def initialize(options, gems) @filter_options_patch = options.keys & %w[filter-major filter-minor filter-patch] - @outdated_gems_by_groups = {} @outdated_gems_list = [] @options_include_groups = [:group, :groups].any? do |v| @@ -92,9 +91,6 @@ def run :current_spec => current_spec, :dependency => dependency, :groups => groups } - - outdated_gems_by_groups[groups] ||= [] - outdated_gems_by_groups[groups] << outdated_gems_list[-1] end if outdated_gems_list.empty? @@ -105,7 +101,7 @@ def run end if options_include_groups - outdated_gems_by_groups.sort.each do |groups, gems| + outdated_gems_list.group_by {|g| g[:groups] }.sort.each do |groups, gems| contains_group = groups.split(", ").include?(options[:group]) next unless options[:groups] || contains_group