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.

Sign up for GitHub

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

Jump to bottom

Refactor lib/annotate.rb #707

Merged
merged 13 commits into from
Dec 30, 2019
Merged

Refactor lib/annotate.rb #707

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
72f3ac6
Copy skip_on_migration?, include_routes?, and include_models? methods
drwl Dec 25, 2019
7a463d2
Make methods point to Helpers class methods
drwl Dec 25, 2019
d880737
Rerun `rubocop --auto-gen-config`
drwl Dec 25, 2019
d7c290b
Replace usages of methods to point to Helpers class instead
drwl Dec 25, 2019
bd5564b
Remove redundant methods from Annotate
drwl Dec 25, 2019
048b7dc
Copy .true? to Annotate::Helpers
drwl Dec 25, 2019
91708a8
Redirect usages of .true? to refer to Helpers.true?
drwl Dec 25, 2019
c6ea1ff
Copy .fallback to Helpers
drwl Dec 25, 2019
73f9aa1
Changes usages of .fallback to use Helpers.fallback instead
drwl Dec 25, 2019
6c1271b
Copy .reset_options into Helpers
drwl Dec 29, 2019
d2b0a55
Move options constants into Constants module
drwl Dec 29, 2019
ef39676
Add Constants to spec_helper
drwl Dec 29, 2019
cdf40c3
Remove .all_options from lib/annotate.rb
drwl Dec 29, 2019
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
26 changes: 12 additions & 14 deletions .rubocop_todo.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-09-02 16:23:56 -0700 using RuboCop version 0.68.1.
# on 2019-12-24 20:33:53 -0800 using RuboCop version 0.68.1.
# 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
Expand Down Expand Up @@ -51,7 +51,7 @@ Layout/AlignArray:
Exclude:
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 107
# Offense count: 111
# Cop supports --auto-correct.
# Configuration parameters: EnforcedHashRocketStyle, EnforcedColonStyle, EnforcedLastArgumentHashStyle.
# SupportedHashRocketStyles: key, separator, table
Expand All @@ -74,7 +74,7 @@ Layout/BlockAlignment:
Exclude:
- 'lib/annotate/annotate_models.rb'

# Offense count: 45
# Offense count: 47
# Cop supports --auto-correct.
Layout/ClosingHeredocIndentation:
Exclude:
Expand All @@ -87,7 +87,7 @@ Layout/ClosingHeredocIndentation:
- 'spec/integration/standalone.rb'
- 'spec/lib/annotate/annotate_models_spec.rb'

# Offense count: 13
# Offense count: 14
# Cop supports --auto-correct.
Layout/EmptyLineAfterGuardClause:
Exclude:
Expand Down Expand Up @@ -173,7 +173,7 @@ Layout/ExtraSpacing:
Layout/IndentFirstHashElement:
EnforcedStyle: consistent

# Offense count: 54
# Offense count: 55
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: auto_detection, squiggly, active_support, powerpack, unindent
Expand Down Expand Up @@ -410,11 +410,11 @@ Lint/ShadowingOuterLocalVariable:
Metrics/AbcSize:
Max: 141

# Offense count: 28
# Offense count: 31
# Configuration parameters: CountComments, ExcludedMethods.
# ExcludedMethods: refine
Metrics/BlockLength:
Max: 259
Max: 268

# Offense count: 1
# Configuration parameters: CountBlocks.
Expand All @@ -439,7 +439,7 @@ Naming/AccessorMethodName:
Exclude:
- 'lib/annotate.rb'

# Offense count: 76
# Offense count: 79
# Configuration parameters: Blacklist.
# Blacklist: (?-mix:(^|\s)(EO[A-Z]{1}|END)(\s|$))
Naming/HeredocDelimiterNaming:
Expand Down Expand Up @@ -572,7 +572,7 @@ Style/FormatStringToken:
Exclude:
- 'lib/annotate/annotate_models.rb'

# Offense count: 186
# Offense count: 189
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: when_needed, always, never
Expand Down Expand Up @@ -633,14 +633,12 @@ Style/MultilineBlockChain:
- 'lib/annotate/annotate_models.rb'
- 'spec/spec_helper.rb'

# Offense count: 5
# Offense count: 3
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: literals, strict
Style/MutableConstant:
Exclude:
- 'lib/annotate.rb'
- 'lib/annotate/annotate_models.rb'
- 'lib/annotate/annotate_routes.rb'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'
- 'spec/integration/rails_2.3_with_bundler/config/environment.rb'
Expand Down Expand Up @@ -778,7 +776,7 @@ Style/StderrPuts:
- 'lib/annotate/annotate_models.rb'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'

# Offense count: 247
# Offense count: 249
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline.
# SupportedStyles: single_quotes, double_quotes
Expand Down Expand Up @@ -830,7 +828,7 @@ Style/UnneededPercentQ:
- 'annotate.gemspec'
- 'spec/integration/rails_2.3_with_bundler/config/boot.rb'

# Offense count: 465
# Offense count: 477
# Cop supports --auto-correct.
# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
# URISchemes: http, https
Expand Down
33 changes: 6 additions & 27 deletions lib/annotate.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'annotate/annotate_models'
require 'annotate/annotate_routes'
require 'annotate/constants'
require 'annotate/helpers'

begin
# ActiveSupport 3.x...
Expand Down Expand Up @@ -75,10 +76,10 @@ def self.set_defaults(options = {})
#
def self.setup_options(options = {})
POSITION_OPTIONS.each do |key|
options[key] = fallback(ENV[key.to_s], ENV['position'], 'before')
options[key] = Annotate::Helpers.fallback(ENV[key.to_s], ENV['position'], 'before')
end
FLAG_OPTIONS.each do |key|
options[key] = true?(ENV[key.to_s])
options[key] = Annotate::Helpers.true?(ENV[key.to_s])
end
OTHER_OPTIONS.each do |key|
options[key] = !ENV[key.to_s].blank? ? ENV[key.to_s] : nil
Expand All @@ -94,9 +95,9 @@ def self.setup_options(options = {})
options[:wrapper_close] ||= options[:wrapper]

# These were added in 2.7.0 but so this is to revert to old behavior by default
options[:exclude_scaffolds] = Annotate.true?(ENV.fetch('exclude_scaffolds', 'true'))
options[:exclude_controllers] = Annotate.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV.fetch('exclude_scaffolds', 'true'))
options[:exclude_controllers] = Annotate::Helpers.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate::Helpers.true?(ENV.fetch('exclude_helpers', 'true'))

options
end
Expand All @@ -105,18 +106,6 @@ def self.reset_options
all_options.flatten.each { |key| ENV[key.to_s] = nil }
end

def self.skip_on_migration?
ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ Constants::TRUE_RE || ENV['skip_on_db_migrate'] =~ Constants::TRUE_RE
end

def self.include_routes?
ENV['routes'] =~ Constants::TRUE_RE
end

def self.include_models?
ENV['models'] =~ Constants::TRUE_RE
end

def self.loaded_tasks=(val)
@loaded_tasks = val
end
Expand Down Expand Up @@ -191,14 +180,4 @@ def self.bootstrap_rake
load_tasks
Rake::Task[:set_annotation_options].invoke
end

def self.fallback(*args)
args.detect { |arg| !arg.blank? }
end

def self.true?(val)
return false if val.blank?
return false unless val =~ Constants::TRUE_RE
true
end
end
29 changes: 29 additions & 0 deletions lib/annotate/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module Annotate
# Class for holding helper methods. Done to make lib/annotate.rb less bloated.
class Helpers
class << self
def skip_on_migration?
ENV['ANNOTATE_SKIP_ON_DB_MIGRATE'] =~ Constants::TRUE_RE || ENV['skip_on_db_migrate'] =~ Constants::TRUE_RE
end

def include_routes?
ENV['routes'] =~ Constants::TRUE_RE
end

def include_models?
ENV['models'] =~ Constants::TRUE_RE
end

def true?(val)
return false if val.blank?
return false unless val =~ Constants::TRUE_RE

true
end

def fallback(*args)
args.detect { |arg| !arg.blank? }
end
end
end
end
70 changes: 35 additions & 35 deletions lib/tasks/annotate_models.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,46 +11,46 @@ task annotate_models: :environment do
require "#{annotate_lib}/annotate/active_record_patch"

options = {is_rake: true}
ENV['position'] = options[:position] = Annotate.fallback(ENV['position'], 'before')
ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV['position'], 'before')
options[:additional_file_patterns] = ENV['additional_file_patterns'] ? ENV['additional_file_patterns'].split(',') : []
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be mixed in instead of specifying full path? It would also cut back the number of changes needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctran this is done intentionally for now to make it easier to see where everything is coming from. Do you think it would be okay to keep for now?

I'm thinking we could untangle lib/annotate.rb. I'm looking at the Pry gem as a source of inspiration.

Copy link
Owner

@ctran ctran Dec 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine either way, was just wondering if you had specific reasons to do that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it more explicit where it's coming from right now. I'm expecting Helpers to be a temporary solution until it's clearer what the different pieces are.

options[:position_in_class] = Annotate.fallback(ENV['position_in_class'], ENV['position'])
options[:position_in_fixture] = Annotate.fallback(ENV['position_in_fixture'], ENV['position'])
options[:position_in_factory] = Annotate.fallback(ENV['position_in_factory'], ENV['position'])
options[:position_in_test] = Annotate.fallback(ENV['position_in_test'], ENV['position'])
options[:position_in_serializer] = Annotate.fallback(ENV['position_in_serializer'], ENV['position'])
options[:show_foreign_keys] = Annotate.true?(ENV['show_foreign_keys'])
options[:show_complete_foreign_keys] = Annotate.true?(ENV['show_complete_foreign_keys'])
options[:show_indexes] = Annotate.true?(ENV['show_indexes'])
options[:simple_indexes] = Annotate.true?(ENV['simple_indexes'])
options[:position_in_class] = Annotate::Helpers.fallback(ENV['position_in_class'], ENV['position'])
options[:position_in_fixture] = Annotate::Helpers.fallback(ENV['position_in_fixture'], ENV['position'])
options[:position_in_factory] = Annotate::Helpers.fallback(ENV['position_in_factory'], ENV['position'])
options[:position_in_test] = Annotate::Helpers.fallback(ENV['position_in_test'], ENV['position'])
options[:position_in_serializer] = Annotate::Helpers.fallback(ENV['position_in_serializer'], ENV['position'])
options[:show_foreign_keys] = Annotate::Helpers.true?(ENV['show_foreign_keys'])
options[:show_complete_foreign_keys] = Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])
options[:show_indexes] = Annotate::Helpers.true?(ENV['show_indexes'])
options[:simple_indexes] = Annotate::Helpers.true?(ENV['simple_indexes'])
options[:model_dir] = ENV['model_dir'] ? ENV['model_dir'].split(',') : ['app/models']
options[:root_dir] = ENV['root_dir']
options[:include_version] = Annotate.true?(ENV['include_version'])
options[:include_version] = Annotate::Helpers.true?(ENV['include_version'])
options[:require] = ENV['require'] ? ENV['require'].split(',') : []
options[:exclude_tests] = Annotate.true?(ENV['exclude_tests'])
options[:exclude_factories] = Annotate.true?(ENV['exclude_factories'])
options[:exclude_fixtures] = Annotate.true?(ENV['exclude_fixtures'])
options[:exclude_serializers] = Annotate.true?(ENV['exclude_serializers'])
options[:exclude_scaffolds] = Annotate.true?(ENV['exclude_scaffolds'])
options[:exclude_controllers] = Annotate.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_sti_subclasses] = Annotate.true?(ENV['exclude_sti_subclasses'])
options[:ignore_model_sub_dir] = Annotate.true?(ENV['ignore_model_sub_dir'])
options[:format_bare] = Annotate.true?(ENV['format_bare'])
options[:format_rdoc] = Annotate.true?(ENV['format_rdoc'])
options[:format_markdown] = Annotate.true?(ENV['format_markdown'])
options[:sort] = Annotate.true?(ENV['sort'])
options[:force] = Annotate.true?(ENV['force'])
options[:frozen] = Annotate.true?(ENV['frozen'])
options[:classified_sort] = Annotate.true?(ENV['classified_sort'])
options[:trace] = Annotate.true?(ENV['trace'])
options[:wrapper_open] = Annotate.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate.fallback(ENV['wrapper_close'], ENV['wrapper'])
options[:exclude_tests] = Annotate::Helpers.true?(ENV['exclude_tests'])
options[:exclude_factories] = Annotate::Helpers.true?(ENV['exclude_factories'])
options[:exclude_fixtures] = Annotate::Helpers.true?(ENV['exclude_fixtures'])
options[:exclude_serializers] = Annotate::Helpers.true?(ENV['exclude_serializers'])
options[:exclude_scaffolds] = Annotate::Helpers.true?(ENV['exclude_scaffolds'])
options[:exclude_controllers] = Annotate::Helpers.true?(ENV.fetch('exclude_controllers', 'true'))
options[:exclude_helpers] = Annotate::Helpers.true?(ENV.fetch('exclude_helpers', 'true'))
options[:exclude_sti_subclasses] = Annotate::Helpers.true?(ENV['exclude_sti_subclasses'])
options[:ignore_model_sub_dir] = Annotate::Helpers.true?(ENV['ignore_model_sub_dir'])
options[:format_bare] = Annotate::Helpers.true?(ENV['format_bare'])
options[:format_rdoc] = Annotate::Helpers.true?(ENV['format_rdoc'])
options[:format_markdown] = Annotate::Helpers.true?(ENV['format_markdown'])
options[:sort] = Annotate::Helpers.true?(ENV['sort'])
options[:force] = Annotate::Helpers.true?(ENV['force'])
options[:frozen] = Annotate::Helpers.true?(ENV['frozen'])
options[:classified_sort] = Annotate::Helpers.true?(ENV['classified_sort'])
options[:trace] = Annotate::Helpers.true?(ENV['trace'])
options[:wrapper_open] = Annotate::Helpers.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate::Helpers.fallback(ENV['wrapper_close'], ENV['wrapper'])
options[:ignore_columns] = ENV.fetch('ignore_columns', nil)
options[:ignore_routes] = ENV.fetch('ignore_routes', nil)
options[:hide_limit_column_types] = Annotate.fallback(ENV['hide_limit_column_types'], '')
options[:hide_default_column_types] = Annotate.fallback(ENV['hide_default_column_types'], '')
options[:with_comment] = Annotate.true?(ENV['with_comment'])
options[:ignore_unknown_models] = Annotate.true?(ENV.fetch('ignore_unknown_models', 'false'))
options[:hide_limit_column_types] = Annotate::Helpers.fallback(ENV['hide_limit_column_types'], '')
options[:hide_default_column_types] = Annotate::Helpers.fallback(ENV['hide_default_column_types'], '')
options[:with_comment] = Annotate::Helpers.true?(ENV['with_comment'])
options[:ignore_unknown_models] = Annotate::Helpers.true?(ENV.fetch('ignore_unknown_models', 'false'))

AnnotateModels.do_annotations(options)
end
Expand All @@ -64,6 +64,6 @@ task remove_annotation: :environment do
options[:model_dir] = ENV['model_dir']
options[:root_dir] = ENV['root_dir']
options[:require] = ENV['require'] ? ENV['require'].split(',') : []
options[:trace] = Annotate.true?(ENV['trace'])
options[:trace] = Annotate::Helpers.true?(ENV['trace'])
AnnotateModels.remove_annotations(options)
end
6 changes: 3 additions & 3 deletions lib/tasks/annotate_models_migrate.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ module Annotate
@@working = false

def self.update_annotations
unless @@working || Annotate.skip_on_migration?
unless @@working || Annotate::Helpers.skip_on_migration?
@@working = true

self.update_models if Annotate.include_models?
self.update_routes if Annotate.include_routes?
self.update_models if Annotate::Helpers.include_models?
self.update_routes if Annotate::Helpers.include_routes?
end
end

Expand Down
10 changes: 5 additions & 5 deletions lib/tasks/annotate_routes.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ task :annotate_routes => :environment do
require "#{annotate_lib}/annotate/annotate_routes"

options={}
ENV['position'] = options[:position] = Annotate.fallback(ENV['position'], 'before')
options[:position_in_routes] = Annotate.fallback(ENV['position_in_routes'], ENV['position'])
options[:ignore_routes] = Annotate.fallback(ENV['ignore_routes'], nil)
ENV['position'] = options[:position] = Annotate::Helpers.fallback(ENV['position'], 'before')
options[:position_in_routes] = Annotate::Helpers.fallback(ENV['position_in_routes'], ENV['position'])
options[:ignore_routes] = Annotate::Helpers.fallback(ENV['ignore_routes'], nil)
options[:require] = ENV['require'] ? ENV['require'].split(',') : []
options[:wrapper_open] = Annotate.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate.fallback(ENV['wrapper_close'], ENV['wrapper'])
options[:wrapper_open] = Annotate::Helpers.fallback(ENV['wrapper_open'], ENV['wrapper'])
options[:wrapper_close] = Annotate::Helpers.fallback(ENV['wrapper_close'], ENV['wrapper'])
AnnotateRoutes.do_annotations(options)
end

Expand Down
4 changes: 2 additions & 2 deletions spec/lib/annotate/annotate_models_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,12 +790,12 @@ def mock_column(name, type, options = {})

describe '#set_defaults' do
it 'should default show_complete_foreign_keys to false' do
expect(Annotate.true?(ENV['show_complete_foreign_keys'])).to be(false)
expect(Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])).to be(false)
end

it 'should be able to set show_complete_foreign_keys to true' do
Annotate.set_defaults('show_complete_foreign_keys' => 'true')
expect(Annotate.true?(ENV['show_complete_foreign_keys'])).to be(true)
expect(Annotate::Helpers.true?(ENV['show_complete_foreign_keys'])).to be(true)
end

after :each do
Expand Down
Loading