Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
use_packwerk (0.51.1)
use_packwerk (0.52.0)
code_ownership
colorize
package_protections
Expand Down Expand Up @@ -34,14 +34,14 @@ GEM
json (2.6.2)
method_source (1.0.0)
minitest (5.16.2)
package_protections (1.3.0)
package_protections (1.4.0)
activesupport
parse_packwerk
rubocop
rubocop-sorbet
sorbet-runtime
parallel (1.22.1)
parse_packwerk (0.11.0)
parse_packwerk (0.12.0)
sorbet-runtime
parser (3.1.2.0)
ast (~> 2.4.1)
Expand Down Expand Up @@ -83,7 +83,7 @@ GEM
rubocop-ast (>= 1.19.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 1.4.0, < 3.0)
rubocop-ast (1.19.1)
rubocop-ast (1.21.0)
parser (>= 3.1.1.0)
rubocop-sorbet (0.6.11)
rubocop (>= 0.90.0)
Expand Down
15 changes: 2 additions & 13 deletions lib/use_packwerk/private.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,19 +197,7 @@ def self.make_public!(paths_relative_to_root:, per_file_processors:)


file_move_operations = file_paths.map do |path|
parts = path.to_s.split('/')
first_part_of_path = T.must(parts[0])

if Pathname.new(first_part_of_path).dirname.join(ParsePackwerk::PACKAGE_YML_NAME).exist?
package_location = Pathname.new('.')
elsif PERMITTED_PACK_LOCATIONS.include?(first_part_of_path)
package_location = Pathname.new(first_part_of_path).join(T.must(parts[1]))
else
raise StandardError.new('Can only make files in the monolith or in existing packs public')
end

package = ParsePackwerk::Package.from(package_location.join(ParsePackwerk::PACKAGE_YML_NAME))

package = T.must(ParsePackwerk.package_from_path(path))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The above was just a way of finding what package the file belonged to. Now that ParsePackwerk implements this, this could all be removed.

origin_pathname = Pathname.new(path).cleanpath

FileMoveOperation.new(
Expand Down Expand Up @@ -427,6 +415,7 @@ def self.create_pack_if_not_exists!(pack_name:, enforce_privacy:, enforce_depend

sig { params(original_package: ParsePackwerk::Package).returns(ParsePackwerk::Package) }
def self.rewrite_package_with_original_packwerk_values(original_package)
ParsePackwerk.bust_cache!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is because I added a cache to ParsePackwerk.all.

I slightly regret doing that, since it ended up being a bit confusing, and I think I want to change to something closer to what packwerk does later:
rubyatscale/parse_packwerk#9

package_with_protection_defaults = T.must(ParsePackwerk.all.find { |package| package.name == original_package.name })
# PackageProtections also sets `enforce_privacy` and `enforce_dependency` to be true, so we set these back down to their original values
package = ParsePackwerk::Package.new(
Expand Down
40 changes: 20 additions & 20 deletions lib/use_packwerk/private/file_move_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,37 @@ def origin_pack

sig { params(origin_pathname: Pathname, new_package_root: Pathname).returns(Pathname) }
def self.destination_pathname_for_package_move(origin_pathname, new_package_root)
parts = origin_pathname.to_s.split('/')
toplevel_directory = parts[0]
origin_pack = T.must(ParsePackwerk.package_from_path(origin_pathname))

case toplevel_directory.to_s
# This allows us to move files from monolith to packs
when 'app', 'spec', 'lib'
new_implementation = nil
if origin_pack.name == ParsePackwerk::ROOT_PACKAGE_NAME
new_package_root.join(origin_pathname).cleanpath
# This allows us to move files from packs to packs
when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb']
new_package_root.join(T.must(parts[2..]).join('/')).cleanpath
else
raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}")
Pathname.new(origin_pathname.to_s.gsub(origin_pack.name, new_package_root.to_s)).cleanpath

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The old implementation was a bit convoluted because I was basically inferring whether it was the root pack or a parent pack based on the first parts of the directory. It's much more explicit to just swap out the old pack name with the new one! This actually resulted in even less code for the child pack case.

end
end

sig { params(origin_pathname: Pathname).returns(Pathname) }
def self.destination_pathname_for_new_public_api(origin_pathname)
parts = origin_pathname.to_s.split('/')
toplevel_directory = Pathname.new(parts[0])

case toplevel_directory.to_s
# This allows us to make API in the monolith public
when 'app', 'spec'
toplevel_directory.join('public').join(T.must(parts[2..]).join('/')).cleanpath
# This allows us to make API in existing packs public
when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb']
pack_name = Pathname.new(parts[1])
toplevel_directory.join(pack_name).join('app/public').join(T.must(parts[4..]).join('/')).cleanpath
origin_pack = T.must(ParsePackwerk.package_from_path(origin_pathname))
if origin_pack.name == ParsePackwerk::ROOT_PACKAGE_NAME
filepath_without_pack_name = origin_pathname.to_s
else
raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}")
filepath_without_pack_name = origin_pathname.to_s.gsub("#{origin_pack.name}/", '')
end

# We join the pack name with the rest of the path...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similar to above -- I was casing based on whether it was a root or parent pack. This implementation just looks at the path following the pack name, which I think is a lot simpler to understand.

path_parts = filepath_without_pack_name.split("/")
Pathname.new(origin_pack.name).join(
# ... keeping the "app" or "spec"
T.must(path_parts[0]),
# ... substituting "controllers," "services," etc. with "public"
'public',
# ... then the rest is the same
T.must(path_parts[2..]).join("/")
# and we take the cleanpath so `./app/...` becomes `app/...`
).cleanpath
end

sig { returns(FileMoveOperation) }
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@

config.include_context 'app fixtures'

config.around do |example|
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)
Expand Down
Loading