diff --git a/Gemfile.lock b/Gemfile.lock index 462d643..dc74901 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -17,7 +17,7 @@ GIT PATH remote: . specs: - packs (0.0.31) + packs (0.0.32) code_ownership (>= 1.33.0) packs-specification packwerk diff --git a/lib/packs.rb b/lib/packs.rb index cd451ca..78839a9 100644 --- a/lib/packs.rb +++ b/lib/packs.rb @@ -82,7 +82,7 @@ def self.create_pack!( def self.move_to_pack!( pack_name:, paths_relative_to_root: [], - per_file_processors: [] + per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] ) Logging.section('👋 Hi!') do intro = Packs.config.user_event_logger.before_move_to_pack(pack_name) @@ -109,7 +109,7 @@ def self.move_to_pack!( end def self.make_public!( paths_relative_to_root: [], - per_file_processors: [] + per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] ) Logging.section('Making files public') do intro = Packs.config.user_event_logger.before_make_public @@ -163,7 +163,7 @@ def self.add_dependency!( def self.move_to_parent!( pack_name:, parent_name:, - per_file_processors: [] + per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] ) Logging.section('👋 Hi!') do intro = Packs.config.user_event_logger.before_move_to_parent(pack_name) diff --git a/lib/packs/cli.rb b/lib/packs/cli.rb index 61d3dd3..393f6d5 100644 --- a/lib/packs/cli.rb +++ b/lib/packs/cli.rb @@ -74,10 +74,7 @@ def list_top_violations(type, pack_name = nil) LONG_DESC sig { params(paths: String).void } def make_public(*paths) - Packs.make_public!( - paths_relative_to_root: paths, - per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] - ) + Packs.make_public!(paths_relative_to_root: paths) exit_successfully end @@ -92,8 +89,7 @@ def make_public(*paths) def move(pack_name, *paths) Packs.move_to_pack!( pack_name: pack_name, - paths_relative_to_root: paths, - per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] + paths_relative_to_root: paths ) exit_successfully end @@ -129,9 +125,21 @@ def update end desc 'get_info [ packs/my_pack packs/my_other_pack ]', 'Get info about size and violations for packs' + option :include_date, type: :boolean, default: false, aliases: :d, banner: "Include today's date as part of the data (useful to take snapshots over time)" + option :format, type: :string, default: 'detail', aliases: :f, banner: 'Specify the output format (detail, csv)' + option :types, type: :string, default: 'privacy,dependency', aliases: :t, banner: 'List of validation types to include (privacy,dependency,architecture)' sig { params(pack_names: String).void } def get_info(*pack_names) - Private.get_info(packs: parse_pack_names(pack_names)) + selected_types = options[:types].to_s.downcase.split(',') + invalid_types = selected_types - POSIBLE_TYPES + raise StandardError, "Invalid type(s): #{invalid_types.join(', ')}. Possible types are: #{POSIBLE_TYPES.join(', ')}" unless invalid_types.empty? + + Private.get_info( + packs: parse_pack_names(pack_names), + format: options[:format].to_sym, + types: selected_types.map(&:to_sym), + include_date: options[:include_date] + ) exit_successfully end @@ -147,8 +155,7 @@ def rename def move_to_parent(child_pack_name, parent_pack_name) Packs.move_to_parent!( parent_name: parent_pack_name, - pack_name: child_pack_name, - per_file_processors: [Packs::RubocopPostProcessor.new, Packs::CodeOwnershipPostProcessor.new] + pack_name: child_pack_name ) exit_successfully end diff --git a/lib/packs/private.rb b/lib/packs/private.rb index 7eb5198..91a2588 100644 --- a/lib/packs/private.rb +++ b/lib/packs/private.rb @@ -10,6 +10,8 @@ require 'packs/private/pack_relationship_analyzer' require 'packs/private/interactive_cli' +require 'date' + module Packs module Private extend T::Sig @@ -197,13 +199,20 @@ def self.move_to_parent!( new_dependencies += [new_package_name] end + new_config = other_package.config.dup + if new_config['ignored_dependencies'] + new_config['ignored_dependencies'] = new_config['ignored_dependencies'].map do |d| + d == pack_name ? new_package_name : d + end + end + new_other_package = ParsePackwerk::Package.new( name: other_package.name, enforce_privacy: other_package.enforce_privacy, enforce_dependencies: other_package.enforce_dependencies, dependencies: new_dependencies.uniq.sort, metadata: other_package.metadata, - config: other_package.config + config: new_config ) ParsePackwerk.write_package_yml!(new_other_package) @@ -479,45 +488,97 @@ def self.packwerk_package_to_pack(package) Packs.find(package.name) end - sig { params(packs: T::Array[Packs::Pack]).void } - def self.get_info(packs: Packs.all) - inbound_violations = {} - outbound_violations = {} + sig do + params( + packs: T::Array[Packs::Pack], + format: Symbol, + types: T::Array[Symbol], + include_date: T::Boolean + ).void + end + def self.get_info(packs: Packs.all, format: :detail, types: %i[privacy dependency architecture], include_date: false) + require 'csv' if format == :csv + + today = Date.today.iso8601 + violations = { + inbound: {}, + outbound: {} + } + ParsePackwerk.all.each do |p| p.violations.each do |violation| - outbound_violations[p.name] ||= [] - outbound_violations[p.name] << violation - inbound_violations[violation.to_package_name] ||= [] - inbound_violations[violation.to_package_name] << violation + violations[:outbound][p.name] ||= [] + violations[:outbound][p.name] << violation + violations[:inbound][violation.to_package_name] ||= [] + violations[:inbound][violation.to_package_name] << violation end end - all_inbound = T.let([], T::Array[ParsePackwerk::Violation]) - all_outbound = T.let([], T::Array[ParsePackwerk::Violation]) + all = { + inbound: T.let([], T::Array[ParsePackwerk::Violation]), + outbound: T.let([], T::Array[ParsePackwerk::Violation]) + } packs.each do |pack| - all_inbound += inbound_violations[pack.name] || [] - all_outbound += outbound_violations[pack.name] || [] + all[:inbound] += violations[:inbound][pack.name] || [] + all[:outbound] += violations[:outbound][pack.name] || [] end - puts "There are #{all_inbound.select(&:privacy?).sum { |v| v.files.count }} total inbound privacy violations" - puts "There are #{all_inbound.select(&:dependency?).sum { |v| v.files.count }} total inbound dependency violations" - puts "There are #{all_outbound.select(&:privacy?).sum { |v| v.files.count }} total outbound privacy violations" - puts "There are #{all_outbound.select(&:dependency?).sum { |v| v.files.count }} total outbound dependency violations" + case format + when :csv + headers = ['Date', 'Pack name', 'Owned by', 'Size', 'Public API'] + headers.delete('Date') unless include_date + types.each do |type| + headers << "Inbound #{type} violations" + headers << "Outbound #{type} violations" + end + puts CSV.generate_line(headers) + else # :detail + puts "Date: #{today}" if include_date + types.each do |type| + inbound_count = all[:inbound].select { _1.type.to_sym == type }.sum { |v| v.files.count } + outbound_count = all[:outbound].select { _1.type.to_sym == type }.sum { |v| v.files.count } + puts "There are #{inbound_count} total inbound #{type} violations" + puts "There are #{outbound_count} total outbound #{type} violations" + end + end packs.sort_by { |p| -p.relative_path.glob('**/*.rb').count }.each do |pack| - puts "\n=========== Info about: #{pack.name}" - owner = CodeOwnership.for_package(pack) - puts "Owned by: #{owner.nil? ? 'No one' : owner.name}" - puts "Size: #{pack.relative_path.glob('**/*.rb').count} ruby files" - puts "Public API: #{pack.relative_path.join('app/public')}" - - inbound_for_pack = inbound_violations[pack.name] || [] - outbound_for_pack = outbound_violations[pack.name] || [] - puts "There are #{inbound_for_pack.select(&:privacy?).sum { |v| v.files.count }} inbound privacy violations" - puts "There are #{inbound_for_pack.flatten.select(&:dependency?).sum { |v| v.files.count }} inbound dependency violations" - puts "There are #{outbound_for_pack.select(&:privacy?).sum { |v| v.files.count }} outbound privacy violations" - puts "There are #{outbound_for_pack.flatten.select(&:dependency?).sum { |v| v.files.count }} outbound dependency violations" + + row = { + date: today, + pack_name: pack.name, + owner: owner.nil? ? 'No one' : owner.name, + size: pack.relative_path.glob('**/*.rb').count, + public_api: pack.relative_path.join('app/public') + } + + row.delete(:date) unless include_date + + types.each do |type| + key = ['inbound', type, 'violations'].join('_').to_sym + row[key] = (violations[:inbound][pack.name] || []).select { _1.type.to_sym == type }.sum { |v| v.files.count } + key = ['outbound', type, 'violations'].join('_').to_sym + row[key] = (violations[:outbound][pack.name] || []).select { _1.type.to_sym == type }.sum { |v| v.files.count } + end + + case format + when :csv + puts CSV.generate_line(row.values) + else # :detail + puts "\n=========== Info about: #{row[:pack_name]}" + + puts "Date: #{row[:date]}" if include_date + puts "Owned by: #{row[:owner]}" + puts "Size: #{row[:size]} ruby files" + puts "Public API: #{row[:public_api]}" + types.each do |type| + key = ['inbound', type, 'violations'].join('_').to_sym + puts "There are #{row[key]} inbound #{type} violations" + key = ['outbound', type, 'violations'].join('_').to_sym + puts "There are #{row[key]} outbound #{type} violations" + end + end end end diff --git a/lib/packs/private/interactive_cli/use_cases/get_info.rb b/lib/packs/private/interactive_cli/use_cases/get_info.rb index f80537c..dfe50d8 100644 --- a/lib/packs/private/interactive_cli/use_cases/get_info.rb +++ b/lib/packs/private/interactive_cli/use_cases/get_info.rb @@ -26,9 +26,23 @@ def perform!(prompt) selected_packs = PackSelector.single_or_all_pack_multi_select(prompt, question_text: 'What pack(s) would you like info on?') end + format = prompt.select('What output format do you want?', %w[Detail CSV]) + + types = prompt.multi_select( + 'What violation types do you want stats for?', + %w[Privacy Dependency Architecture] + ) + + include_date = !prompt.no?('Should the current date be included in the report?') + puts "You've selected #{selected_packs.count} packs. Wow! Here's all the info." - Private.get_info(packs: selected_packs) + Private.get_info( + packs: selected_packs, + format: format.downcase.to_sym, + types: types.map(&:downcase).map(&:to_sym), + include_date: include_date + ) end end end diff --git a/packs.gemspec b/packs.gemspec index 3502937..0d5a418 100644 --- a/packs.gemspec +++ b/packs.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'packs' - spec.version = '0.0.31' + spec.version = '0.0.32' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index 8da49d0..4041f5d 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -1124,6 +1124,21 @@ def write_codeownership_config expect(ParsePackwerk.find('packs/fruits').dependencies).to eq(['packs/fruits/apples']) end + it 'updates ignored_dependencies in all package.yml' do + write_package_yml('packs/fruits') + write_package_yml('packs/apples', dependencies: ['packs/other_pack'], metadata: { 'custom_field' => 'custom value' }) + write_package_yml('packs/turtles', config: { 'ignored_dependencies' => ['packs/apples'] }) + + Packs.move_to_parent!( + pack_name: 'packs/apples', + parent_name: 'packs/fruits' + ) + + ParsePackwerk.bust_cache! + + expect(ParsePackwerk.find('packs/turtles').config['ignored_dependencies']).to eq(['packs/fruits/apples']) + end + it 'gives some helpful output to users' do logged_output = '' @@ -1229,6 +1244,36 @@ def write_codeownership_config expect(ParsePackwerk.find('packs/fruits').dependencies).to eq(['packs/fruits/apples']) end end + + describe 'RubocopPostProcessor' do + it 'modifies an application-specific file, .rubocop_todo.yml, correctly' do + write_file('.rubocop.yml') + + write_file('.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/foo/app/services/foo.rb + CONTENTS + + before_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) + + expect(before_rubocop_todo).to eq({ 'Layout/BeginEndAlignment' => { 'Exclude' => ['packs/foo/app/services/foo.rb'] } }) + + write_file('packs/foo/app/services/foo.rb') + Packs.create_pack!(pack_name: 'packs/bar') + Packs.create_pack!(pack_name: 'packs/foo') + ParsePackwerk.bust_cache! + Packs.move_to_parent!( + pack_name: 'packs/foo', + parent_name: 'packs/bar', + per_file_processors: [Packs::RubocopPostProcessor.new] + ) + + after_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) + expect(after_rubocop_todo).to eq({ 'Layout/BeginEndAlignment' => { 'Exclude' => ['packs/bar/foo/app/services/foo.rb'] } }) + end + end end describe 'lint_package_todo_yml_files!' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f99b58a..8476f6e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,7 +38,8 @@ enforce_privacy: T::Boolean, visible_to: T::Array[String], metadata: T.untyped, - owner: T.nilable(String) + owner: T.nilable(String), + config: T::Hash[String, T.untyped] ).void end def write_package_yml( @@ -48,7 +49,8 @@ def write_package_yml( enforce_privacy: true, visible_to: [], metadata: {}, - owner: nil + owner: nil, + config: {} ) if owner metadata.merge!({ 'owner' => owner }) @@ -60,7 +62,7 @@ def write_package_yml( enforce_dependencies: enforce_dependencies, enforce_privacy: enforce_privacy, metadata: metadata, - config: {} + config: config ) ParsePackwerk.write_package_yml!(package)