From e3114928eb14939e1f58f34d22e8d01546a7db03 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Thu, 29 Dec 2022 12:54:05 -0500 Subject: [PATCH] Incorporate "packs" into use_packs (#72) * Use packs/rspec/support * bump version * Change implementation to use packs * rubocop --- Gemfile.lock | 12 ++- lib/use_packs.rb | 17 ++-- lib/use_packs/cli.rb | 4 +- .../code_ownership_post_processor.rb | 3 +- lib/use_packs/private.rb | 19 ++++- .../private/interactive_cli/pack_selector.rb | 12 +-- .../interactive_cli/use_cases/get_info.rb | 9 +- .../interactive_cli/use_cases/visualize.rb | 7 +- ...p@1.28.0.rbi => code_ownership@1.29.2.rbi} | 13 ++- sorbet/rbi/gems/packs@0.0.5.rbi | 84 +++++++++++++++++++ ...ks@0.0.31.rbi => rubocop-packs@0.0.33.rbi} | 19 ++--- spec/spec_helper.rb | 20 +---- use_packs.gemspec | 3 +- 13 files changed, 158 insertions(+), 64 deletions(-) rename sorbet/rbi/gems/{code_ownership@1.28.0.rbi => code_ownership@1.29.2.rbi} (96%) create mode 100644 sorbet/rbi/gems/packs@0.0.5.rbi rename sorbet/rbi/gems/{rubocop-packs@0.0.31.rbi => rubocop-packs@0.0.33.rbi} (91%) diff --git a/Gemfile.lock b/Gemfile.lock index 30814c6..bf839a0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -17,9 +17,10 @@ GIT PATH remote: . specs: - use_packs (0.0.12) + use_packs (0.0.13) code_ownership colorize + packs packwerk parse_packwerk rubocop-packs @@ -52,9 +53,9 @@ GEM smart_properties builder (3.2.4) byebug (11.1.3) - code_ownership (1.29.1) + code_ownership (1.29.2) code_teams (~> 1.0) - parse_packwerk + packs sorbet-runtime code_teams (1.0.0) sorbet-runtime @@ -77,6 +78,8 @@ GEM nokogiri (1.13.10) mini_portile2 (~> 2.8.0) racc (~> 1.4) + packs (0.0.5) + sorbet-runtime parallel (1.22.1) parse_packwerk (0.18.0) sorbet-runtime @@ -130,8 +133,9 @@ GEM unicode-display_width (>= 1.4.0, < 3.0) rubocop-ast (1.21.0) parser (>= 3.1.1.0) - rubocop-packs (0.0.32) + rubocop-packs (0.0.33) activesupport + packs parse_packwerk rubocop rubocop-sorbet diff --git a/lib/use_packs.rb b/lib/use_packs.rb index 402b4d1..7c74bd7 100644 --- a/lib/use_packs.rb +++ b/lib/use_packs.rb @@ -282,16 +282,19 @@ def self.lint_package_todo_yml_files! end end - sig { params(packs: T::Array[ParsePackwerk::Package]).void } + sig { params(packs: T::Array[Packs::Pack]).void } def self.lint_package_yml_files!(packs) packs.each do |p| + packwerk_package = ParsePackwerk.find(p.name) + next if packwerk_package.nil? + new_package = ParsePackwerk::Package.new( - name: p.name, - enforce_privacy: p.enforce_privacy, - enforce_dependencies: p.enforce_dependencies, - dependencies: p.dependencies.uniq.sort, - metadata: p.metadata, - config: p.config + name: packwerk_package.name, + enforce_privacy: packwerk_package.enforce_privacy, + enforce_dependencies: packwerk_package.enforce_dependencies, + dependencies: packwerk_package.dependencies.uniq.sort, + metadata: packwerk_package.metadata, + config: packwerk_package.config ) ParsePackwerk.write_package_yml!(new_package) end diff --git a/lib/use_packs/cli.rb b/lib/use_packs/cli.rb index 361cfe1..f75f37a 100644 --- a/lib/use_packs/cli.rb +++ b/lib/use_packs/cli.rb @@ -118,9 +118,9 @@ def regenerate_rubocop_todo(*pack_names) # This is used by thor to know that these private methods are not intended to be CLI commands no_commands do - sig { params(pack_names: T::Array[String]).returns(T::Array[ParsePackwerk::Package]) } + sig { params(pack_names: T::Array[String]).returns(T::Array[Packs::Pack]) } def parse_pack_names(pack_names) - pack_names.empty? ? ParsePackwerk.all : pack_names.map { |p| ParsePackwerk.find(p.gsub(%r{/$}, '')) }.compact + pack_names.empty? ? Packs.all : pack_names.map { |p| Packs.find(p.gsub(%r{/$}, '')) }.compact end end end diff --git a/lib/use_packs/code_ownership_post_processor.rb b/lib/use_packs/code_ownership_post_processor.rb index 9c0156a..65f8d4f 100644 --- a/lib/use_packs/code_ownership_post_processor.rb +++ b/lib/use_packs/code_ownership_post_processor.rb @@ -34,7 +34,8 @@ def before_move_file!(file_move_operation) @teams << 'Unknown' end - if !CodeOwnership.for_package(file_move_operation.destination_pack).nil? + pack = Packs.find(file_move_operation.destination_pack.name) + if pack && !CodeOwnership.for_package(pack).nil? CodeOwnership.remove_file_annotation!(relative_path_to_origin.to_s) @did_move_files = true end diff --git a/lib/use_packs/private.rb b/lib/use_packs/private.rb index baba3fe..a9a1d0b 100644 --- a/lib/use_packs/private.rb +++ b/lib/use_packs/private.rb @@ -378,7 +378,8 @@ def self.create_pack_if_not_exists!(pack_name:, enforce_privacy:, enforce_depend ) ParsePackwerk.write_package_yml!(package) - RuboCop::Packs.set_default_rubocop_yml(packs: [package]) + pack = Packs.find(package.name) + RuboCop::Packs.set_default_rubocop_yml(packs: [pack].compact) current_contents = package.yml.read new_contents = current_contents.gsub('MyTeam', 'MyTeam # specify your team here, or delete this key if this package is not owned by one team') @@ -452,6 +453,22 @@ def self.write_package_todo_to_tmp_folder(package_todo_files, tmp_folder) temp_package_todo_yml.write(contents) end end + + sig { params(packages: T::Array[ParsePackwerk::Package]).returns(T::Array[Packs::Pack]) } + def self.packwerk_packages_to_packs(packages) + packs = [] + packages.each do |package| + pack = Packs.find(package.name) + packs << pack if !pack.nil? + end + + packs + end + + sig { params(package: ParsePackwerk::Package).returns(T.nilable(Packs::Pack)) } + def self.packwerk_package_to_pack(package) + Packs.find(package.name) + end end private_constant :Private diff --git a/lib/use_packs/private/interactive_cli/pack_selector.rb b/lib/use_packs/private/interactive_cli/pack_selector.rb index 1656323..6930815 100644 --- a/lib/use_packs/private/interactive_cli/pack_selector.rb +++ b/lib/use_packs/private/interactive_cli/pack_selector.rb @@ -6,9 +6,9 @@ module InteractiveCli class PackSelector extend T::Sig - sig { params(prompt: TTY::Prompt, question_text: String).returns(ParsePackwerk::Package) } + sig { params(prompt: TTY::Prompt, question_text: String).returns(Packs::Pack) } def self.single_pack_select(prompt, question_text: 'Please use space to select a pack') - packs = ParsePackwerk.all.to_h { |t| [t.name, t] } + packs = Packs.all.to_h { |t| [t.name, t] } pack_selection = T.let(prompt.select( question_text, @@ -16,7 +16,7 @@ def self.single_pack_select(prompt, question_text: 'Please use space to select a filter: true, per_page: 10, show_help: :always - ), T.nilable(ParsePackwerk::Package)) + ), T.nilable(Packs::Pack)) while pack_selection.nil? prompt.error( @@ -29,15 +29,15 @@ def self.single_pack_select(prompt, question_text: 'Please use space to select a pack_selection end - sig { params(prompt: TTY::Prompt, question_text: String).returns(T::Array[ParsePackwerk::Package]) } + sig { params(prompt: TTY::Prompt, question_text: String).returns(T::Array[Packs::Pack]) } def self.single_or_all_pack_multi_select(prompt, question_text: 'Please use space to select one or more packs') pack_selection = T.let(prompt.multi_select( question_text, - ParsePackwerk.all.to_h { |t| [t.name, t] }, + Packs.all.to_h { |t| [t.name, t] }, filter: true, per_page: 10, show_help: :always - ), T::Array[ParsePackwerk::Package]) + ), T::Array[Packs::Pack]) while pack_selection.empty? prompt.error( diff --git a/lib/use_packs/private/interactive_cli/use_cases/get_info.rb b/lib/use_packs/private/interactive_cli/use_cases/get_info.rb index 6ed8c68..ebecd19 100644 --- a/lib/use_packs/private/interactive_cli/use_cases/get_info.rb +++ b/lib/use_packs/private/interactive_cli/use_cases/get_info.rb @@ -20,7 +20,7 @@ def perform!(prompt) if team_or_pack == 'By team' teams = TeamSelector.multi_select(prompt) - selected_packs = ParsePackwerk.all.select do |p| + selected_packs = Packs.all.select do |p| teams.map(&:name).include?(CodeOwnership.for_package(p)&.name) end else @@ -51,12 +51,13 @@ def perform!(prompt) 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" - selected_packs.sort_by { |p| -p.directory.glob('**/*.rb').count }.each do |pack| + selected_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.directory.glob('**/*.rb').count} ruby files" - puts "Public API: #{pack.directory.join('app/public')}" + 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] || [] diff --git a/lib/use_packs/private/interactive_cli/use_cases/visualize.rb b/lib/use_packs/private/interactive_cli/use_cases/visualize.rb index 248dc94..5ac5784 100644 --- a/lib/use_packs/private/interactive_cli/use_cases/visualize.rb +++ b/lib/use_packs/private/interactive_cli/use_cases/visualize.rb @@ -22,14 +22,17 @@ def perform!(prompt) by_name_or_by_owner = prompt.select('Do you select packs by name or by owner?', ['By name', 'By owner']) if by_name_or_by_owner == 'By owner' teams = TeamSelector.multi_select(prompt) - selected_packs = ParsePackwerk.all.select do |p| + selected_packs = Packs.all.select do |p| teams.map(&:name).include?(CodeOwnership.for_package(p)&.name) end else selected_packs = PackSelector.single_or_all_pack_multi_select(prompt) end - VisualizePackwerk.package_graph!(selected_packs) + packwerk_packages = selected_packs.map do |pack| + T.must(ParsePackwerk.find(pack.name)) + end + VisualizePackwerk.package_graph!(packwerk_packages) end end diff --git a/sorbet/rbi/gems/code_ownership@1.28.0.rbi b/sorbet/rbi/gems/code_ownership@1.29.2.rbi similarity index 96% rename from sorbet/rbi/gems/code_ownership@1.28.0.rbi rename to sorbet/rbi/gems/code_ownership@1.29.2.rbi index 340e7e9..be71eac 100644 --- a/sorbet/rbi/gems/code_ownership@1.28.0.rbi +++ b/sorbet/rbi/gems/code_ownership@1.29.2.rbi @@ -23,9 +23,12 @@ module CodeOwnership sig { params(file: ::String).returns(T.nilable(::CodeTeams::Team)) } def for_file(file); end - sig { params(package: ::ParsePackwerk::Package).returns(T.nilable(::CodeTeams::Team)) } + sig { params(package: ::Packs::Pack).returns(T.nilable(::CodeTeams::Team)) } def for_package(package); end + sig { params(team: T.any(::CodeTeams::Team, ::String)).returns(::String) } + def for_team(team); end + sig { params(files: T::Array[::String], autocorrect: T::Boolean, stage_changes: T::Boolean).void } def validate!(files: T.unsafe(nil), autocorrect: T.unsafe(nil), stage_changes: T.unsafe(nil)); end @@ -41,6 +44,7 @@ end class CodeOwnership::Cli class << self def for_file(argv); end + def for_team(argv); end def run!(argv); end private @@ -193,13 +197,8 @@ class CodeOwnership::Private::OwnershipMappers::PackageOwnership sig { override.params(files: T::Array[::String]).returns(T::Hash[::String, T.nilable(::CodeTeams::Team)]) } def map_files_to_owners(files); end - sig { params(package: ::ParsePackwerk::Package).returns(T.nilable(::CodeTeams::Team)) } + sig { params(package: ::Packs::Pack).returns(T.nilable(::CodeTeams::Team)) } def owner_for_package(package); end - - private - - sig { params(file: ::String).returns(T.nilable(::ParsePackwerk::Package)) } - def map_file_to_relevant_package(file); end end class CodeOwnership::Private::OwnershipMappers::TeamGlobs diff --git a/sorbet/rbi/gems/packs@0.0.5.rbi b/sorbet/rbi/gems/packs@0.0.5.rbi new file mode 100644 index 0000000..1fceaca --- /dev/null +++ b/sorbet/rbi/gems/packs@0.0.5.rbi @@ -0,0 +1,84 @@ +# typed: true + +# DO NOT EDIT MANUALLY +# This is an autogenerated file for types exported from the `packs` gem. +# Please instead update this file by running `bin/tapioca gem packs`. + +module Packs + class << self + sig { returns(T::Array[::Packs::Pack]) } + def all; end + + sig { void } + def bust_cache!; end + + sig { returns(::Packs::Private::Configuration) } + def config; end + + sig { params(blk: T.proc.params(arg0: ::Packs::Private::Configuration).void).void } + def configure(&blk); end + + sig { params(name: ::String).returns(T.nilable(::Packs::Pack)) } + def find(name); end + + sig { params(file_path: T.any(::Pathname, ::String)).returns(T.nilable(::Packs::Pack)) } + def for_file(file_path); end + + private + + sig { returns(T::Array[::Pathname]) } + def package_glob_patterns; end + + sig { returns(T::Hash[::String, ::Packs::Pack]) } + def packs_by_name; end + end +end + +Packs::PACKAGE_FILE = T.let(T.unsafe(nil), String) + +class Packs::Pack < ::T::Struct + const :name, ::String + const :path, ::Pathname + const :raw_hash, T::Hash[T.untyped, T.untyped] + const :relative_path, ::Pathname + + sig { returns(::String) } + def last_name; end + + sig { returns(T::Hash[T.untyped, T.untyped]) } + def metadata; end + + sig { params(relative: T::Boolean).returns(::Pathname) } + def yml(relative: T.unsafe(nil)); end + + class << self + sig { params(package_yml_absolute_path: ::Pathname).returns(::Packs::Pack) } + def from(package_yml_absolute_path); end + + def inherited(s); end + end +end + +module Packs::Private + class << self + sig { returns(::Pathname) } + def root; end + end +end + +class Packs::Private::Configuration < ::T::Struct + prop :pack_paths, T::Array[::String] + + class << self + sig { returns(::Packs::Private::Configuration) } + def fetch; end + + def inherited(s); end + + sig { params(config_hash: T::Hash[T.untyped, T.untyped]).returns(T::Array[::String]) } + def pack_paths(config_hash); end + end +end + +Packs::Private::Configuration::CONFIGURATION_PATHNAME = T.let(T.unsafe(nil), Pathname) +Packs::Private::Configuration::DEFAULT_PACK_PATHS = T.let(T.unsafe(nil), Array) diff --git a/sorbet/rbi/gems/rubocop-packs@0.0.31.rbi b/sorbet/rbi/gems/rubocop-packs@0.0.33.rbi similarity index 91% rename from sorbet/rbi/gems/rubocop-packs@0.0.31.rbi rename to sorbet/rbi/gems/rubocop-packs@0.0.33.rbi index de1f9f5..753b051 100644 --- a/sorbet/rbi/gems/rubocop-packs@0.0.31.rbi +++ b/sorbet/rbi/gems/rubocop-packs@0.0.33.rbi @@ -50,12 +50,12 @@ class RuboCop::Cop::Packs::RootNamespaceIsPackName::DesiredZeitwerkApi sig do params( relative_filename: ::String, - package_for_path: ::ParsePackwerk::Package + package_for_path: ::Packs::Pack ).returns(T.nilable(::RuboCop::Cop::Packs::RootNamespaceIsPackName::DesiredZeitwerkApi::NamespaceContext)) end def for_file(relative_filename, package_for_path); end - sig { params(pack: ::ParsePackwerk::Package).returns(::String) } + sig { params(pack: ::Packs::Pack).returns(::String) } def get_pack_based_namespace(pack); end private @@ -63,9 +63,6 @@ class RuboCop::Cop::Packs::RootNamespaceIsPackName::DesiredZeitwerkApi sig { params(remaining_file_path: ::String, package_name: ::String).returns(::String) } def get_actual_namespace(remaining_file_path, package_name); end - sig { params(pack: ::ParsePackwerk::Package).returns(::String) } - def get_package_last_name(pack); end - sig { returns(::Pathname) } def root_pathname; end end @@ -165,10 +162,10 @@ module RuboCop::Packs sig { params(root_pathname: ::String).returns(::String) } def pack_based_rubocop_config(root_pathname: T.unsafe(nil)); end - sig { params(packs: T::Array[::ParsePackwerk::Package], files: T::Array[::String]).void } + sig { params(packs: T::Array[::Packs::Pack], files: T::Array[::String]).void } def regenerate_todo(packs: T.unsafe(nil), files: T.unsafe(nil)); end - sig { params(packs: T::Array[::ParsePackwerk::Package]).void } + sig { params(packs: T::Array[::Packs::Pack]).void } def set_default_rubocop_yml(packs:); end sig { returns(T::Array[::String]) } @@ -215,13 +212,13 @@ module RuboCop::Packs::Private sig { returns(T::Array[T::Hash[T.untyped, T.untyped]]) } def rubocop_todo_ymls; end - sig { params(package: ::ParsePackwerk::Package).returns(T::Array[::String]) } + sig { params(package: ::Packs::Pack).returns(T::Array[::String]) } def validate_failure_mode_strict(package); end - sig { params(package: ::ParsePackwerk::Package).returns(T::Array[::String]) } + sig { params(package: ::Packs::Pack).returns(T::Array[::String]) } def validate_rubocop_todo_yml(package); end - sig { params(package: ::ParsePackwerk::Package).returns(T::Array[::String]) } + sig { params(package: ::Packs::Pack).returns(T::Array[::String]) } def validate_rubocop_yml(package); end end end @@ -253,7 +250,7 @@ class RuboCop::Packs::Private::Offense < ::T::Struct const :cop_name, ::String const :filepath, ::String - sig { returns(::ParsePackwerk::Package) } + sig { returns(T.nilable(::Packs::Pack)) } def pack; end class << self diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c1e5f57..e6773f5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,8 @@ # frozen_string_literal: true require 'use_packs' -require 'tmpdir' +require 'packs' +require 'packs/rspec/support' RSpec.configure do |config| # Enable flags like --only-failures and --next-failure @@ -19,27 +20,10 @@ ParsePackwerk.bust_cache! example.run end - - config.around do |example| - prefix = [File.basename($0), Process.pid].join('-') # rubocop:disable Style/SpecialGlobalVars - tmpdir = Dir.mktmpdir(prefix) - Dir.chdir(tmpdir) do - example.run - end - ensure - FileUtils.rm_rf(T.must(tmpdir)) - end end extend T::Sig # rubocop:disable Style/MixinUsage: -sig { params(path: String, content: String).returns(Integer) } -def write_file(path, content = '') - pathname = Pathname.new(path) - FileUtils.mkdir_p(pathname.dirname) - pathname.write(content) -end - sig do params( pack_name: String, diff --git a/use_packs.gemspec b/use_packs.gemspec index e6b9d31..f4ee043 100644 --- a/use_packs.gemspec +++ b/use_packs.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'use_packs' - spec.version = '0.0.12' + spec.version = '0.0.13' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] @@ -30,6 +30,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'code_ownership' spec.add_dependency 'colorize' + spec.add_dependency 'packs' spec.add_dependency 'packwerk' spec.add_dependency 'parse_packwerk' spec.add_dependency 'rubocop-packs'