Replies: 7 comments 9 replies
-
At gusto we have discussed this as the problem of "not all violations are created equal" - i.e., we care about certain (cross-domain) violations more than others: (within-domain) violations. I also find it interesting to ask... what if we succeed in cleaning up billing? Let's imaging we do and make it its own app. Then packwerk/pks would work as is! With this framing in mind, here are two ideas we have bounced around:
There is a bunch of subtleties going on with both and they have slightly different capabilities but try to solve the same problem. Aside: wildcard dependency specifications (and more generally wildcards in all packwerk and packwerk-extensions list configs) solve are nice in this context and generally in large applications. I feel like adding those is a no-regrets move... |
Beta Was this translation helpful? Give feedback.
-
I like the idea of Aggregated Dependency Enforcement. It feels pragmatic. I imagine some people will want to see explicit dependencies of subpackages and this approach sounds like it would remain backwards compatible.
Does this overload "enforce_dependencies"? The recent #384 merge makes me think there's an alternative where a new top-level key like "inherit_dependencies" becomes relevant. It has the downside of only being situationally relevant (when in a nested structure) but I think the distinct representation could be better than introducing more values to "enforce_dependencies". |
Beta Was this translation helpful? Give feedback.
-
It may be convenient to have nested packages inherit parent dependencies by default, because it would be easy to forget to specify that explicitly each time for nested packs. And allow nested packages to have dependencies of their own. |
Beta Was this translation helpful? Give feedback.
-
Thanks for starting this discussion! I've always thought of packwerk as basically doing this:
It makes total sense to me to introduce more sophisticated ways to constrain package graphs that reflect how folks think about design in the real world. I'm open to us experimenting with different interfaces for doing this. I think that the options you presented @exterm are sharp tools – they give users to express the concepts of nested packages without explicitly saying that it's a nested package per se. This provides a lot more flexibility to the end user, maybe at the expense of simplicity. I do think aggregated/inherited dependency configuration is closer to explicitly calling something a sub-package. I also really like @astyagun 's idea of treating nested packs as first-class citizens. So if a user nests package B under package A, then without any other configuration, a couple things are true:
Then maybe nested packs can override or configure that default behavior as necessary... I'm not sure what use cases folks had in mind though. |
Beta Was this translation helpful? Give feedback.
-
As a counterargument to these proposals, I've heard feedback that Packwerk does dependency management better than many other tools because there isn't any magic / transitivity. The list of dependencies you see are exactly (statically) what a package needs, and code can be extracted with only this list in mind. At the cost of duplication, we have a good amount of simplicity. I'm not 100% against these proposed features, but I think it might compromise this simplicity. At Shopify, we've seen a few use-cases where we have nested packages that have entirely different dependencies than their parent. Are we implying in this discussion that this is an anti-pattern? I think |
Beta Was this translation helpful? Give feedback.
-
It may be easier to think about this if problems with current implementation are visualized. Here's what I can think of: Absence of a "circular dependency" error---
title: 1
---
flowchart LR
subgraph A
B
end
B-->C
C-->A
---
title: 2
---
flowchart LR
subgraph A
B
end
A-->C
C-->B
---
title: 3
---
flowchart LR
subgraph A
B
C
end
B-->D
D-->C
|
Beta Was this translation helpful? Give feedback.
-
If these are problems to solve, then another possible solution is to inherit dependencies (outgoing and incoming) from nested packs to parent packs. But try to mark inherited dependencies somehow, so that they affect dependencies checker, but are not drawn on graphs by tools like Graphwerk and visualize_packs. |
Beta Was this translation helpful? Give feedback.
-
When packwerk was created, the simple definition - all files in a directory with a package.yml files and all subdirs belong to that package - did not take into account nested packages.
The implementation ended up treating nested packages as just different packages, with no relationship to the parent package whatsoever.
This means that if you declare code in a parent package to be in its own, nested package, it's at the same time removed from the parent package.
I find that this leads to a conflict between two goals:
Imagine this setup (Rails folder structure omitted for brevity):
Now if we decide that we want ownership and encapsulation for
billing/buyers
. We drop in apackage.yml
:billing/buyers/bill_buyers.rb
is now no longer part of thebilling
package, for all intents and purposes. It can have arbitrary dependencies on packages outside ofbilling
that may be opposite of what its parent package has. This problem is especially apparent if the parent package expresses an architectural layer.Aside: Gusto implemented an architecture checker that partially solves this problem for layers. But it doesn't follow packwerk conventions that package structure is reflected in the directory structure (layers are not packages, and thus don't have to be directories), and adds a different kind of violations that are called
architecture
but really are just dependency violations. In doing so, it increases complexity of the overall setup.As a reminder, this is what an application's architecture may look like (not intended to depict an ideal situation):
I want to propose two changes that could help resolve this tension in a backwards compatible way. Note that these changes are independent of each other; we can do either without the other, both, or none.
One proposal concerns the way we express dependencies, the other concerns the scope in which dependencies are enforced.
Wildcard Dependency Specs
TL;DR:
Depend on a package and all its nested packages. This would mean that a consumer is not impacted by restructuring in what it depends on.
There's a slight problem with this one: How do we "hide" code? Maybe that means we shouldn't do this. Maybe it just means that existing privacy checkers need some adjustment.
Aggregated Dependency Enforcement
This is the real heart of this proposal.
TL;DR:
This would mean that for dependency enforcement purposes, files in this package are still part of the parent package. It means that if
billing/buyers/bill_buyers.rb
references a constant inplatform/auth.rb
,billing/package.yml
needs to specify a dependency onplatform
. Essentially, it means that dependency enforcement is delegated to the parent package.When dependency enforcement is delegated to the parent, the package doing the delegation may not specify its own
dependencies
.I'm not set on the name
parent
and open for suggestions. "nested"? "inherited"? "merge_with_parent"?Notes
I'm a bit jealous of more expressive architecture enforcement tools like Python's
import-linter
, but we have to remember that Ruby is not Python; it doesn't have a built-in mechanism equivalent to Python's module imports, so we're starting at a different level.If we wanted similar expressiveness as
import-linter
, I think we'd have to redesignpackwerk
from the bottom up. This proposal, in contrast, is about incremental improvements on top of what we have in place, and what we have existing momentum on.Beta Was this translation helpful? Give feedback.
All reactions