Open squahtx opened 3 years ago
Also suggest we make this a CI check, assuming it's not too expensive to run.
Really, we should have better rules about what is allowed to import from where. For example: synapse.handlers
is not allowed to import from synapse.rest
. Currently, it's all rather ad-hoc.
I did a test today and got only failures in one file:
************* Module synapse.spam_checker_api.__init__
Your code has been rated at 9.99/10
Really, we should have better rules about what is allowed to import from where. For example:
synapse.handlers
is not allowed to import fromsynapse.rest
. Currently, it's all rather ad-hoc.
Something like https://github.com/seddonym/import-linter/ might help enforce this sort of rule.
(I had https://sourcery.ai/blog/dependency-rules/ in mind having previously seen it on https://news.ycombinator.com/item?id=33999191, but this seems to be some kind of paid for product?)
Cyclic imports can cause unit tests to fail when run in isolation, or CI for Synapse modules to fail: https://github.com/matrix-org/synapse-email-account-validity/runs/3979337154?check_suite_focus=true
A full list can be obtained using
pylint --disable=all --enable R0401 synapse
(thanks to @DMRobertson)As of 85a09f8b8ba7c8023c0d28a526d32111fc704197, the current list of cyclic imports is: