Shopify / packwerk

Good things come in small packages.
MIT License
1.54k stars 111 forks source link

Add assocaitions_exclude to exclude files from association inspection #380

Closed gmcgibbon closed 2 months ago

gmcgibbon commented 8 months ago

What are you trying to accomplish?

Fixes #370

What approach did you choose and why?

Other gems like ActiveModel::Serializers define association methods that reference serializers. We can ignore these files entirely with an exclude pattern.

What should reviewers focus on?

The exclude pattern breaks public API, and we only get guarantees from Sorbet if the plugin package uses it (and they don't have to). So, I added an ArgumentError check to make sure all we still support the old signatures without relative_file. We could alternatively proceed without this precaution, so I can drop the commit if needed.

Type of Change

Additional Release Notes

Include any notes here to include in the release description. For example, if you selected "breaking change" above, leave notes on how users can transition to this version.

If no additional notes are necessary, delete this section or leave it unchanged.

Checklist

exterm commented 8 months ago

Could it be an option to allow users to define a path glob for ignoring associations in? ...I'm not sure which solution I'd prefer... but I am also not sure that it makes sense to take on this complexity in packwerk itself.

Maybe the whole association handling could be moved into a plugin, which could then also handle serializers?

gmcgibbon commented 6 months ago

@rafaelfranca does this approach align with your request for changes?

zumkorn commented 2 months ago

@gmcgibbon @rafaelfranca is there any reason why we can't merge this PR?

gmcgibbon commented 2 months ago

@zumkorn no, thanks for the reminder. I'll merge now!

@rafaelfranca if you have any additional feedback let's fix forward.