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.

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

Enable spring by default when executing packwerk init #153

Merged
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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ Or install it yourself as:

$ gem install packwerk

3. Run `packwerk init` to generate the configuration files
2. Run `bundle binstub packwerk` to generate the binstub
3. Run `bin/packwerk init` to generate the configuration files
exterm marked this conversation as resolved.
Show resolved Hide resolved

## Usage

Expand Down
6 changes: 3 additions & 3 deletions TROUBLESHOOT.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ Packwerk can give feedback via continuous integration (CI) if you have it set up

You can specify folders or packages in Packwerk commands for a shorter run time:

packwerk check components/your_package
bin/packwerk check components/your_package

packwerk update-deprecations components/your_package
bin/packwerk update-deprecations components/your_package

_Note: You cannot specify folders or packages for `packwerk validate` because the command runs for the entire application._
_Note: You cannot specify folders or packages for `bin/packwerk validate` because the command runs for the entire application._

![](static/packwerk_check_violation.gif)

Expand Down
25 changes: 17 additions & 8 deletions USAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ The [package principles](https://en.wikipedia.org/wiki/Package_principles) page

## Getting started

After including Packwerk in the Gemfile, you can generate the necessary files to get Packwerk running by executing:
After including Packwerk in the Gemfile, you will first want to generate a binstub:
You can do this by running `bundle binstub packwerk`, which will generate a [binstub](https://bundler.io/man/bundle-binstubs.1.html#DESCRIPTION) at `bin/packwerk`.

packwerk init
Then, you can generate the necessary files to get Packwerk running by executing:

bin/packwerk init

Here is a list of files generated:

Expand All @@ -69,6 +72,12 @@ Packwerk reads from the `packwerk.yml` configuration file in the root directory.
| custom_associations | N/A | list of custom associations, if any |
| parallel | true | when true, fork code parsing out to subprocesses |

## Setting up Spring

[Spring](https://github.com/rails/spring) is a preloader for Rails. Because `packwerk` loads `Rails`, it can be sped up dramatically by enabling spring. Packwerk supports the usage of Spring.
Firstly, spring needs to know about the packwerk spring command when spring is loading. To do that, add `require 'packwerk/spring_command'` to `config/spring.rb` in your application.
Secondly, to enable Spring, first run `bin/spring binstub packwerk` which will "springify" the generated binstub.

### Using a custom ERB parser

You can specify a custom ERB parser if needed. For example, if you're using `<%graphql>` tags from https://github.com/github/graphql-client in your ERBs, you can use a custom parser subclass to comment them out so that Packwerk can parse the rest of the file:
Expand Down Expand Up @@ -134,7 +143,7 @@ Any new inflectors should be added to `config/inflections.yml`.

There are some criteria that an application must meet in order to have a valid package system. These criteria include having a valid autoload path cache, package definition files, and application folder structure. The dependency graph within the package system also has to be acyclic.

We recommend setting up the package system validation for your Rails application in a CI step (or through a test suite for Ruby projects) separate from `packwerk check`.
We recommend setting up the package system validation for your Rails application in a CI step (or through a test suite for Ruby projects) separate from `bin/packwerk check`.

Use the following command to validate the application:

Expand Down Expand Up @@ -237,7 +246,7 @@ You can also specify folders for a shorter run time:

![](static/packwerk_check.gif)

In order to keep the package system valid at each version of the application, we recommend running `packwerk check` in your CI pipeline.
In order to keep the package system valid at each version of the application, we recommend running `bin/packwerk check` in your CI pipeline.

See: [TROUBLESHOOT.md - Sample violations](TROUBLESHOOT.md#Sample-violations)

Expand All @@ -247,17 +256,17 @@ For existing codebases, packages are likely to have existing boundary violations

If so, you will want to stop the bleeding and prevent more violations from occuring. The existing violations in the codebase can be recorded in a [deprecated references list](#Understanding_the_list_of_deprecated_references) by executing:

packwerk update-deprecations
bin/packwerk update-deprecations

Similar to `packwerk check`, you may also run `packwerk update-deprecations` on folders or packages:
Similar to `bin/packwerk check`, you may also run `bin/packwerk update-deprecations` on folders or packages:

packwerk update-deprecations components/your_package
bin/packwerk update-deprecations components/your_package

![](static/packwerk_update.gif)

_Note: Changing dependencies or enabling dependencies will not require a full update of the codebase, only the package that changed. On the other hand, changing or enabling privacy will require a full update of the codebase._

`packwerk update-deprecations` should only be run to record existing violations and to remove deprecated references that have been worked off. Running `packwerk update-deprecations` to resolve a violation should be the very last resort.
`bin/packwerk update-deprecations` should only be run to record existing violations and to remove deprecated references that have been worked off. Running `bin/packwerk update-deprecations` to resolve a violation should be the very last resort.

See: [TROUBLESHOOT.md - Troubleshooting violations](TROUBLESHOOT.md#Troubleshooting_violations)

Expand Down
8 changes: 7 additions & 1 deletion exe/packwerk
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,10 @@

require "packwerk"

Packwerk::Cli.new(style: Packwerk::OutputStyles::Coloured.new).run(ARGV.dup)
# Needs to be run in test environment in order to have test helper paths available in the autoload paths
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what the previous binstub template did

ENV["RAILS_ENV"] = "test"

# Command line arguments needs to be duplicated because spring modifies it
packwerk_argv = ARGV.dup
cli = Packwerk::Cli.new(style: Packwerk::OutputStyles::Coloured.new)
cli.run(packwerk_argv)
2 changes: 1 addition & 1 deletion lib/packwerk/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def generate_configs
result = if success
<<~EOS

🎉 Packwerk is ready to be used. You can start defining packages and run `packwerk check`.
🎉 Packwerk is ready to be used. You can start defining packages and run `bin/packwerk check`.
For more information on how to use Packwerk, see: https://github.com/Shopify/packwerk/blob/main/USAGE.md
EOS
else
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/deprecated_references.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def dump
#
# You can regenerate this file using the following command:
#
# packwerk update-deprecations #{@package.name}
# bin/packwerk update-deprecations #{@package.name}
MESSAGE
File.open(@filepath, "w") do |f|
f.write(message)
Expand Down
28 changes: 28 additions & 0 deletions lib/packwerk/spring_command.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true
# typed: false

require "spring/commands"

module Packwerk
class SpringCommand
def env(*)
# Packwerk needs to run in a test environment, which has a set of autoload paths that are
# often a superset of the dev/prod paths (for example, test/support/helpers)
"test"
end

def exec_name
"packwerk"
end

def gem_name
"packwerk"
end

def call
load(Gem.bin_path(gem_name, exec_name))
end
end

Spring.register_command("packwerk", SpringCommand.new)
end
20 changes: 20 additions & 0 deletions test/unit/spring_command_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# typed: ignore
# frozen_string_literal: true

require "test_helper"
require "spring/commands"

module Packwerk
class SpringCommandTest < Minitest::Test
test "registers command with Spring when loaded" do
require "packwerk/spring_command"

command = Spring.command("packwerk")

assert_not_nil(command, message: "packwerk command not registered with Spring")
assert_equal("packwerk", command.exec_name)
assert_equal("packwerk", command.gem_name)
assert_equal("test", command.env(nil))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you copied this test over from the previous PR, right?

Do you think it is useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, copied it over. I think it's probably not very useful.... but I think it's basically like a syntax + interface implementation test. This loads packwerk/spring_command so it will at least catch some simple mistakes. I think it can't hurt to keep.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. The test is not very useful but as we don't expect the command to change often, it also doesn't incur a huge cost.

end
end