alexevanczuk / packs

A pure Rust implementation of packwerk, a gradual modularization tool for Ruby
MIT License
69 stars 7 forks source link

Should packs have an 'init' command? #144

Open stevegeek opened 7 months ago

stevegeek commented 7 months ago

Thanks for your work on this, amazing speed difference checking a large project!

Just was creating a new rails app as a reproduction for a separate issue I am having with packs. I thought I could just use packs without packwerk installed, but I dont see an init command to setup configuration for me (eg root pack yml). It would be nice to be able to init the root pack and packwerk.yml directly using packs cause I guess then you wouldn't need packwerk at all?

stevegeek commented 7 months ago

In relation to the issue I was creating a repro for, I guess it is unsupported behaviour. We are using automatic_namespaces but packs readme says zeitwerk default namespaces are not support, so Im guessing thats the issue Im having.

alexevanczuk commented 7 months ago

Hey @stevegeek thank you for opening this issue!

Actually @mclark just added support for automatic namespaces for GitHub.

If you're feeling brave, want to try it out and update the docs to remove the not supported bit and add directions for use?

Similarly regarding the init question -- you're right this functionality doesn't exist. Would be happy to accept a contribution for this functionality! Alternatively, setting up packwerk is really just a matter of adding a packwerk.yml, the format of which you can find in the packwerk repo (or here within the test suite).

Let me know if you have any issues setting up packwerk or pks in the meantime while we don't have support for this. Would be happy to help. We could also just have written directions in the interim period.

stevegeek commented 7 months ago

@alexevanczuk thanks for the detailed reply! I will def see if the latest changes help with the issue I was having. And could try my hand at implementing the init command if I find some time

stevegeek commented 7 months ago

Happy to say I think the latest version fixes the issue for me!

stephen$ time bin/packwerk check packs/api_filter_query/
📦 Packwerk is inspecting 43 files
...........................................
📦 Finished in 7.09 seconds

No offenses detected
No stale violations detected

real    0m10.250s
user    0m6.969s
sys     0m22.673s

to

stephen$ time pks check packs/api_filter_query/
No violations detected!

real    0m0.200s
user    0m0.088s
sys     0m1.252s
stevegeek commented 7 months ago

Actually sorry, I was checking the wrong pack! I still see some unexpected violations. packs is picking up a definition of a class from another pack rather than a class with the same name defined in the lexical scope.

I see from list-definitions that pks says "::Schemas::User" is defined at "packs/api_schemas/app/public/schemas/user.rb" but that should be ApiSchemas::Schemas::User with automatic_pack_namespace: true which we use.

Looking at the code to see if I can work out how we might add https://github.com/gap777/automatic_namespaces type support to pks, ie so one can do

metadata:
  automatic_pack_namespace: true

and be able to skip repeating the module name in the directory structure

alexevanczuk commented 7 months ago

Hey @stevegeek correct -- we will need to add support for automatic namespaces. Check out some of the PRs that @mclark recently merged -- I think with his work we should pretty easily be able to also parse for the metadata required by automatic namespaces and set that as the default namespace.

How do ya feel about trying to throw together a PR? If not, I can look into adding this!