rubyatscale / packs Goto Github PK

After creating a pack with the command create the file package.yml is created inside the new pack with the owner in the main level of the YML.

enforce_dependencies: true
enforce_privacy: true
owner: MyTeam

When running the validate command, it doesn't recognize the key owner:

Validation failed ❗

❓ Malformed syntax in the following manifests:

Unknown keys: ["owner"] in "/Users/diogogallo/workspace/x/domains/test/package.yml"

Looking at the private.rb the command below should not add owner as config but as metadata.

if Pathname.new('config/code_ownership.yml').exist?
          config = {
            'owner' => team.nil? ? 'MyTeam' : team.name
          }
        else
          config = {}
        end

Adding the metadata before the owner it works:

enforce_dependencies: true
enforce_privacy: true
metadata:
  owner: MyTeam

The Language Server Protocol allows for real time feedback in code.

The packs gem could provide its dependency violations as LSP diagnostic feedback, which would allow real time feedback on where someone might introduce a dependency by accident.

I see that there is already something similar in the works for vs code.
But having that function as a service, that speaks the LSP would make this available to a wide range of editors, including VS Code, NeoVim, Emacs and more.

Playing around with a minimal Gemfile (for a workshop) I deleted rubocop from the gemfile and now bin/packs commands fails.

Ideally, use_packs makes use of rubocop if it is there and ignores it if it is not.

@shageman has released v0.5.x of visualize_packs with lots of changes but that version breaks compatibility with previous versions. In particular, I'm getting an error when using bin/packs visualize with packs 0.0.24 and visualize_packs 0.5.3 coming from this line.

/home/oboxodo/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/visualize_packs-0.5.3/lib/visualize_packs.rb:11:in `package_graph!': wrong number of arguments (given 1, expected 3) (ArgumentError)
	from /home/oboxodo/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/packs-0.0.24/lib/packs/private.rb:592:in `visualize'

Maybe packs should specify < 0.5 for visualize_packs until assing support for the new version.

Trying to use the move command with an absolute path fails because it doesn't resolve the target path right and thinks the target already exists.

Here's an example:

$ use_packwerk move packs/foo /Users/username/projects/myrailsapp/app/models/user.rb
====================================================================================================
👋 Hi!

You are moving a file to a pack, which is great. Check out https://go/packwerk for more info!

Please bring any questions or issues you have in your development process to #ruby-modularity or #product-infrastructure.
We'd be happy to try to help through pairing, accepting feedback, changing our process, changing our tools, and more.

====================================================================================================
File Operations

[SKIP] Not moving /Users/username/projects/myrailsapp/app/models/user.rb, /Users/username/projects/myrailsapp/app/models/user.rb already exists
[SKIP] Not moving /Users/username/projects/myrailsapp/spec/models/user_spec.rb, /Users/username/projects/myrailsapp/spec/models/user_spec.rb already exists
====================================================================================================
Code Ownership

This section contains info about the current ownership distribution of the moved files.
  Unknown - 2 files
====================================================================================================
Next steps

Your next steps might be:

1) Run `bin/packwerk update-deprecations` to update the violations. Make sure to run `spring stop` if you've added new load paths (new top-level directories) in your pack.

2) Update TODO lists for rubocop implemented protections. See https://go/packwerk for more info

3) Touch base with each team who owns files involved in this move

4) Expose public API in packs/foo/app/public. Try `bin/use_packwerk make_public packs/foo/path/to/file.rb`

5) Update your readme at packs/foo/README.md

We’re currently moving many packs into folders. We were leveraging the move_to_parent! command in bin/packs and that helped us get going. However, move_to_parent! assumes you are moving a pack under a parent pack. This will then create a package.yml file if it doesn't exist. We don't want to do this.

I see two possible user experiences and three ways to implement this.

1. create a move_to_folder which does everything that move_to_parent does, but does not create a package.yml

2. modify move_to_parent to take an argument about whether to create a package.yml

3. modify move_to_parent to never create a package.yml but modify it if it already exists.

Existing User Experience

Existing options
‣ 1. Create a new pack
  2. Move files
  3. Add a dependency
  4. Get info on one or more packs
  5. Query violations about a pack
  6. Make files or directories public
  7. Move a child pack to be nested under a parent pack
  8. Rename a pack
  9. Run bin/packwerk check
  10. Run bin/packwerk update
  11. Run bin/packwerk validate (detects cycles)
  12. Lint packs/*/package.yml for one or more packs

User Experience: Add a new option

Here's one possible user experience:

Existing options
‣ 1. Create a new pack
  2. Move files
  3. Move a pack <------------- new
  4. Add a dependency
  5. Get info on one or more packs
  6. Query violations about a pack
  7. Make files or directories public
  8. Move a child pack to be nested under a parent pack
  9. Rename a pack
  10. Run bin/packwerk check
  11. Run bin/packwerk update
  12. Run bin/packwerk validate (detects cycles)
  13. Lint packs/*/package.yml for one or more packs

Hello! What would you like to do? Move a pack
Please select a pack: packs/my_pack
Please input the destination directory: 

Pros

• I like how move a pack parallels with move a file

Cons

• I dislike that this will add another option to the bin/packs CLI command which already has 12 options.

User Experience: repurpose an existing option

‣ 1. Create a new pack
  2. Move files
  3. Move a pack <------------- new
  4. Add a dependency
  5. Get info on one or more packs
  6. Query violations about a pack
  7. Make files or directories public
  8. Move a child pack to be nested under a parent pack <-------- deleted
  9. Rename a pack
  10. Run bin/packwerk check
  11. Run bin/packwerk update
  12. Run bin/packwerk validate (detects cycles)
  13. Lint packs/*/package.yml for one or more pack

Hello! What would you like to do? Move a pack to a folder or nested under a parent pack
Please select the pack to move: packs/my_pack
Would you like to 
‣ 1. Move to a folder
  2. Nest under a parent pack
 
# If picking 1
Please input the destination directory: 

# If picking 2
Please select the pack that will be the parent: 

I think User Experience: repurpose an existing option is the winner.

Given this, we could rename move_to_parent to move_pack and have it never create a package.yml but modify it if it already exists.

I realize this is VERY subjective, but I think it would make sense to rename the gem - or at least its CLI executable - to something more elegant. Having _ in the name feels off, very rarely do CLI applications have underscores in their names.

Someone on the Modularity slack mentioned packwerker as an option. This feels more elegant, but I'm afraid this can cause confusion about the relation between packwerk and packwerker.

I'm not proposing a specific name at this point though.

The documentation around configuration mentions a way to override where to look for packs, but it seems like packs create doesn't honor this and only allows for creating packs in certain folders, not the one you override in the configuration.

I understand the configuration file syntax (regexp) doesn't make this easy to figure out where the source of the packs is located at. Could the configuration pack_paths, just always assume nested packs and change from regexp to strings instead?

The default configure file would then be:

packs_paths:
  - gems
  - components
  - packs

and the configuration override example could be:

packs_path:
  - packages
  # or
  - modules/systems

Which would be the same contents as PERMITTED_PACK_LOCATIONS today. Moving forward PERMITTED_PACK_LOCATIONS would likely move to a method that evals the config or uses the default if the config isn't present.

This would allow the create command to just work™️:

./bin/packs create modules/systems/disco-service

Can we add a PR template?
And in the template have a checkbox reminder to bump the version ✅

Hello,

Experimenting with your gem and have a few questions. First of all I added gems:

  gem "packwerk"
  gem "packs"
  gem "packs-rails"

1. packwerk init
2. packs create
3. trying to validate

Questions:

1. I see have this error:

My structure:

2. Unknown keys: ["enforce_privacy"]

3. Can I enforce privacy from my private code and not allow using my class (from packs/pack/services/service.rb) in regular folder app/models for example?

https://github.com/rubyatscale/parse_packwerk has been updated to use "layers" instead of "architecture" but packs has not yet.

#156

For this tool to be adopted widely, it needs to be generic enough to allow anyone to adapt to their needs. Right now, however, when moving files around, it will tell you to about Slack channels that might not exist at your company (among other things that don't make sense).

This behavior should be opt-in and configurable. By default the tool should only output things that apply to everyone.

packs seems a little confused about what is a pack. Packs are packwerk packages that have additional functionality sprinkled on top. Not all packwerk packages are packs, and thus not all packwerk packages have engine-like structure.

This leads to packs move having really surprising behavior when moving files out of a non-pack package, as demonstrated by the regression spec included in this PR. Note that every app has at least one package that is not a pack: The root package. This is currently handled by packs as a special case.

origin_pack = ParsePackwerk.package_from_path(origin_pathname) exemplifies this incorrect assumption: It fetches a package and then assigns it to a variable called pack and proceeds to make assumptions that don't hold for packages that are not packs. Luckily, packs-specification has methods for finding packs, specifically. We should use those.

Hi Team!

I noticed today that this gem has a dependency on the colorize gem which is released under the GPL-2 license. Unfortunately, this license is incompatible with the project I'd like to use packs in. I have two questions (and if this would be better as a discussion I'm happy to close this issue and move it)

1. Would you be open to removing this dependency and replacing it with a gem with a more permissive license or even forgoing the coloring feature all together?
2. My understanding of the GPL-2 license is that any linked source needs to also be released under the same license. packs is MIT, not GPL-2. Is that not a problem for this project as well?

Thank you for your consideration.

When moving a pack to be inside another pack, the code currently searches and replaces for the pack name change inside files that this tooling controls (all the packs package.yml files).

However, when moving pack, we want all references to the old pack name updated including code references to files in the old pack.

I'm proposing we do something like:

`rg 'packs/#{pack}' -l --hidden | xargs sed -i '' 's,packs/#{old_pack_name},packs/#{new_pack_name},g'`

I realize that not all systems will have ripgrep installed.

I was looking at flamegraphs of our app boot time (as one does), and got curious that rubocop was being loaded. I was able to trace it back to:

• lib/packs.rb requires rubocop-packs
• rubocop-packs requires rubocop

I've looked over the code in the repo, and at first glance, it doesn't seem like we're calling anything in that gem, or rubocop for that matter. The closest I can see is Packs::RubocopPostProcessor used in a few places, but even that isn't actually calling rubocop.

I see a few possibilities:

• it is used directly. cool, keep the dependency, but try to make it lazily loaded if possible
• it is used indirectly, or maybe something just needs it required at some point. maybe find a better place to load it?
• it is not used. then it can be removed

Hey!

We have a fairly large modularization effort going on and all the tools you all have created have been very very helpful. Thank you.

I found something I think I want bin/packs to do: merge packs together. We've overgranularized a bit and we've identified some packs that we want smooshed together. Of course it's not terrifically hard to do by hand... but it'd be nice to be able to do it with just a few key strokes.

bin/packs merge packs/auth packs/permissions

This would take files from permissions and put them into auth. If there were any name conflicts (which there shouldn't be in most cases for Zeitwerk to work, i think?) it could fail with a helpful error message. Any dependencies that permissions had that auth didn't have would get copied into the auth/package.yml file.

If you like the idea I might try to add it in a PR. Then again I might be being too optimistic about my time.

We have decided on things that we want to have in our pack READMEs in our organization - and they aren't exactly the same as what is in the template after_create_pack

I think having a way to specify the application after_create_pack and on_create_public_directory_todo messages would be a good addition.

I'll create a PR for this shortly

use_packs has a CLI for interactive (via bin/packs) and non-interactive (via bin/packs some_command).

It's possible for these to drift, which I recently fixed so that every command is available in both interactive and non-interactive modes (except for bin/packs list_top_dependency/privacy_violations, which exists as query in the interactive commands).

There are some changes we could make to improve this.

Namely, if we redesign the commands so that a single file defines both the interactive and non interactive commands (e.g. through some interface), then by design we can ensure both are always populated.

Along with this we should probably add better tests for all commands CLI and non-CLI