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

Proposal: Allow configuring sibling independence #208

Closed
Peter554 opened this issue Nov 21, 2023 · 6 comments
Closed

Proposal: Allow configuring sibling independence #208

Peter554 opened this issue Nov 21, 2023 · 6 comments

Comments

@Peter554
Copy link
Contributor

Peter554 commented Nov 21, 2023

In layer contracts it is possible to define sibling layers. These sibling layers are always required to be independent. I believe it would be helpful to allow sibling independence to be configured, on a per layer basis. This would allow us to express contracts where we don't care about inter-layer imports.

The existing syntax for independent sibling layers is:

foo | bar | baz

A possible/proposed syntax for non-independent sibling layers could be:

foo : bar : baz

A mixed syntax would be disallowed and cause an error

// not allowed
foo | bar : baz

The work of enforcing layer contracts is mostly handed in grimp, so work to achieve this would have to start there. A draft / proof of concept can be found here -> seddonym/grimp#136. If this proposal is approved I would be happy to polish those grimp changes and then afterwards make the appropriate importlinter changes.

@seddonym
Copy link
Owner

Thanks for this. I'm not sure... having potential circular dependencies between three modules doesn't sound like the kind of contract I'd personally want. On the other hand, perhaps Import Linter shouldn't be too opinionated about things like that.

I'll have a think about it.

@Peter554
Copy link
Contributor Author

Peter554 commented Nov 24, 2023

Thanks. My imagined use case is something like:

foo : bar : baz
application
domain1 : domain2
data

Where:

  • We want to have an exhaustive, layered contract (exhaustive is very useful).
  • But perhaps we have some higher layers foo, bar and baz and it's also not really clear what the ordering of foo, bar and baz should be right now. For some reason it's non-trivial to refactor things such that they all live under one package (in an ideal world foo, bar and baz should probably all be part of a single package (interfaces maybe 😉) that is a single layer, but maybe there is some good reason why that is not something that is easy to do).
  • Likewise, perhaps the domain has been split across multiple packages domain1 and domain2, and for some unknown reason it's also non-trivial to combine these.

Without allowing the option to configure sibling independence, it makes it difficult to create layered contracts that are exhaustive and capture all the information we want. Of course, in an ideal world and starting a project from scratch, one would probably not choose the architecture above, but for an existing project of size that has something like this going on, I think it's better to have some contract than none, and ideally the most expressive and powerful one we can have. From my perspective I think importlinter should not really be too opinionated about good vs. bad architecture, since we can't know the (perhaps-good) reasons why users projects have the strange architectures they have, but should rather just provide a tool to enforce a configured architecture.

@seddonym
Copy link
Owner

I think you make a good case for this! I also like the : syntax. Happy to consider a PR for it, and let me know if you want to discuss implementation at all.

@Peter554
Copy link
Contributor Author

I think you make a good case for this! I also like the : syntax.

Great 😃

Happy to consider a PR for it, and let me know if you want to discuss implementation at all.

I've had a go at the implementation in grimp here => seddonym/grimp#136 It's ready for review 🤞.

@seddonym
Copy link
Owner

seddonym commented Jan 9, 2024

This is now released in Import Linter v2.0. Thanks for all your help!

@seddonym seddonym closed this as completed Jan 9, 2024
@Peter554
Copy link
Contributor Author

Peter554 commented Jan 9, 2024

This is now released in Import Linter v2.0. Thanks for all your help!

Great, thanks very much for taking the time to support with this 🙂

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

No branches or pull requests

2 participants