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

Group offenses together #8

Open
wildmaples opened this issue Sep 25, 2020 · 3 comments
Open

Group offenses together #8

wildmaples opened this issue Sep 25, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@wildmaples
Copy link
Contributor

We currently can dump a lot of information at the user:

/Some/path/foo.rb:136:8
Privacy violation: '::Foo::Bar' is private to 'foo' but referenced from 'path'.
Is there a public entrypoint in 'foo/app/public/' that you can use instead?
Inference details: 'Foo::Bar' refers to ::Foo::Bar which seems to be defined in foo/app/foo/models/bar.rb.

/Some/path/foo.rb:151:8
Privacy violation: '::Foo::Spam' is private to 'foo' but referenced from 'path'.
Is there a public entrypoint in 'foo/app/public/' that you can use instead?
Inference details: 'Foo::Spam' refers to ::Foo::Spam which seems to be defined in foo/app/models/foo/spam.rb.

/Some/path/foo.rb:161:8
Privacy violation: '::Foo::Eggs' is private to 'foo' but referenced from 'path'.
Is there a public entrypoint in 'foo/app/public/' that you can use instead?
Inference details: 'Foo::Eggs' refers to ::Foo::Eggs which seems to be defined in foo/app/models/foo/eggs.rb.

There's a huge amount of repetition in there, and that's mostly because we were trying to fit the rubocop mold. Let's investigate grouping these together in a more concise and informative way.

Here's one idea:

# Privacy violations

These references appear to reference a private constant in relevant packages. You should look for a publicly accessible constant (<link> to learn more).

- /Some/path/foo.rb:136:8 references `Foo::Bar` in package 'foo'
- /Some/path/foo.rb:151:8 references `Foo::Spam` in package 'foo'
- /Some/path/foo.rb:161:8 references 'Foo::Eggs' in package 'foo'

# Dependency violations

...
@wildmaples wildmaples added the enhancement New feature or request label Sep 25, 2020
@exterm
Copy link
Contributor

exterm commented Oct 1, 2020

Should probably be implemented as an additional output formatter, so that we can switch between both formats and allow users to keep the old format if they depend on it.

shioyama pushed a commit that referenced this issue May 21, 2021
@Epigene
Copy link

Epigene commented Sep 7, 2022

This would definitely be useful.
I'm only using packwerk for dependency enforcement, so here's my two cents - implement some packwerk check --concise flag that changes output from:

packs/awards/app/graphql/events/award/options/mutations/reorder.rb:20:14
Dependency violation: ::Order belongs to 'app/services', but 'packs/awards/app/graphql' does not specify a dependency on 'app/services'.
Are we missing an abstraction?
Is the code making the reference, and the referenced constant, in the right packages?

Inference details: this is a reference to ::Order which seems to be defined in app/services/order.rb.
To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations


packs/awards/app/graphql/events/award/options/mutations/reorder.rb:21:14
Dependency violation: ::User belongs to 'app/models', but 'packs/awards/app/graphql' does not specify a dependency on 'app/models'.
Are we missing an abstraction?
Is the code making the reference, and the referenced constant, in the right packages?

Inference details: this is a reference to ::User which seems to be defined in app/models/user.rb.
To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations


packs/awards/app/graphql/events/award/options_resolver.rb:27:10
Dependency violation: ::Filters::FilteredScope belongs to 'app/services', but 'packs/awards/app/graphql' does not specify a dependency on 'app/services'.
Are we missing an abstraction?
Is the code making the reference, and the referenced constant, in the right packages?

Inference details: this is a reference to ::Filters::FilteredScope which seems to be defined in app/services/filters/filtered_scope.rb.
To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations

to:

'packs/awards/app/graphql' unspecified dependencies:
  - 'app/models'
  - 'app/services'  

This way it's straightforward to judge which pack(s) have dependency problems, especially after moving existing code to a new pack or when defining sub-packs.

@alexevanczuk
Copy link
Contributor

@Epigene Something to consider if you want this new format sooner or you want it to say something very specific to your org is that you can create your own offenses formatter and pass that in. At Gusto, this is what we do to layer in more specific Gusto information like the team that owns the package in question. Let me know if you need any help with this!

Separately, out of curiosity, are you making your models and services separate packs? We have divided up our app by domain, including putting models for a domain in its domain pack, so would be interesting to chat more and share strategies. You're welcome to join the Ruby/Rails Modularity Slack Server if you want to chat more!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants