Disable the "acyclic dependency graph" check?

[mrcasals]

Hi!

We wanted to use the dependencies list in our package.yml files to build a tree of direct dependencies, but after generating the correct data we got an error while running bin/packwerk validate:

Validation failed ❗
Expected the package dependency graph to be acyclic, but it contains the following cycles:

Why are you checking this? Would it make sense to add a configuration option to disable this check?

[gmcgibbon]

Hello!

Why are you checking this?

We're making sure that the dependency graph of your application doesn't contain packages that have circular references to other packages. From a design point of view, if you have a circular dependency, you are likely wanting two packages to be one bigger package, or wanting the circularly referenced code to be moved to a separate intermediary package both other packages depend on.

Would it make sense to add a configuration option to disable this check?

I personally don't think so, we don't want to encourage cyclic dependencies in applications, because it makes the dependency graph feed into itself (eg. if Package A depends on Package B, but Package B also depends on Package A, how would you separate them into two different applications? Would there even be a point to drawing these package boundaries?).

[alexevanczuk]

Hi @mrcasals – I generally agree with @gmcgibbon 's sentiment that the intent of dependencies is to be an acyclic graph of dependencies.

That beings said, at Gusto, folks often use enforce_dependencies when they truly do want to enforce most dependencies.

However, there are some pernicious dependencies that are entangled within a piece of the code that are not just the main focus area. That is – they want to enforce some but not all dependencies to keep focused.

The current solution to this problem is one of:
(A) just add the violations always. This doesn’t feel great because it encourages bad habits and worsens signal/noise ratio.
(B) attempt to just add the dependency to package.yml. Not only does this sometimes just not work because it creates cycles, but it overloads the dependencies list. No longer does it represent all the things someone truly wants to depend on, but things that folks want to not think about right now.

My proposal is to have an ignored_dependencies key. As the name suggests, it’s just a list of dependencies that will never produce dependency violations. It functions like dependencies insofar as no violation is produced, but cycles are permitted.

I think it’d add another dimension of gradularity to this tool – not only can people gradually modularize, but they can more gradually focus on specific pack-to-pack connections.

Would love to hear folks' thoughts on this and if this solves your specific problem!

[alexevanczuk]

Implementation: #350

[gmcgibbon]

at Gusto, folks often use enforce_dependencies when they truly do want to enforce most dependencies.

However, there are some pernicious dependencies that are entangled within a piece of the code that are not just the main focus area.

Can you elaborate on what this scenario is? My initial question would be why an entangled package has dependencies enabled in the first place. Maybe it doesn't, because apparently cyclic checks don't check for enforcement. Perhaps the solution here is to just disable cyclic detection for packages that have dependency enforcement turned off?

[shageman]

[coworker of Alex here]

What Alex is mentioning is that there are packages where we have found the number of dependency violations to be so overwhelming for a package that, despite the fact that we would love to remove them all, we can't focus on them at the moment. We have the rubyatscale deeply integrated into CI and if they didn't turn enforce_dependencies off, they would get bugged basically on every single PR.

If you've ever seen a god model in a Rails app and imagine that to be its own package (because you want to clean up its relation to the rest of the app, then you likely can imagine this scenario.

Perhaps the solution here is to just disable cyclic detection for packages that have dependency enforcement turned off?

Remember that cycles are only prevented for accepted dependencies. And those imply that you want this dependency. I can see how it is a bit confusing that you may not enforce dependencies but still check for acyclicality of the accepted dependency graph. However, these two things are different and separate. Accepting dependencies as you mention are the "design point of view." Enforcing actual dependencies is the feedback loop based on what the app is doing today. So, its future design vs structure today. I believe, we don't want future designs to allow for a pattern that is considered an anti pattern. That's irrespective of whether you currently want feedback regarding dependencies you have.

[gmcgibbon]

There seems to be a fair bit of chatter on the PR now, so I'll shift my focus to there. Thank you for giving me more context. I agree cyclic validations on non-enforcing packages and what you're speaking of are separate issues.

[alexevanczuk]

Based on discussion here and in the PR above, I opened up a new discussion proposing cycles as a TODO item: #353

Let me know what you all think! Also @mrcasals let me know if it would meet your need.