pytest-dev / pluggy

A minimalist production ready plugin system
https://pluggy.readthedocs.io/en/latest/
MIT License
1.21k stars 122 forks source link

Class diagram #341

Open bluetech opened 2 years ago

bluetech commented 2 years ago

All of the Hook* classes are somewhat confusing to me, and I always have to rediscover how they relate. So I created an ASCII diagram of it. I think it's worth shoving in a comment somewhere, or in some "architecture" document. I'll use this issue as a placeholder for that.

                                                 ┌──────────┐
                                                 │ HookSpec │
                                                 └──────────┘
                                                       ▲       plugin1    ┌────────┐
                                                       │  ┌──────────────►│HookImpl│
                                                       │  │               └────────┘
                                       foo_hook1 ┌─────┴──┴─┐
                                      ┌──────────┤HookCaller├───►...
                                      │          └────────┬─┘
                                      │                   │               ┌────────┐
                                      │                   └──────────────►│HookImpl│
                                      │                        plugin2    └────────┘
    ┌─────────────┐  pm.hook  ┌───────┴─┐
    │PluginManager├──────────►│HookRelay├───►...
    └─────────────┘           └───────┬─┘
                                      │                        plugin2    ┌────────┐
                                      │                  ┌───────────────►│HookImpl│
                                      │                  │                └────────┘
                                      │          ┌───────┴──┐
                                      └─────────►│HookCaller├───►...
                                       foo_hook2 └─────┬─┬──┘
                                                       │ │                ┌────────┐
                                                       │ └───────────────►│HookImpl│
                                                       ▼       plugin3    └────────┘
                                                 ┌──────────┐
                                                 │ HookSpec │
                                                 └──────────┘
RonnyPfannschmidt commented 2 years ago

Maybe we should plntuml this

bluetech commented 2 years ago

Just saw this https://github.com/github/roadmap/issues/372 which is similar to planetuml, I might try it.

Leaving their example here to see when it actually works :)

graph TD;
    A-->B;
    A-->C;
    B-->D;
    C-->D;
RonnyPfannschmidt commented 2 years ago

sounds fun, will wait for that then,

Northxw commented 2 years ago

All of the Hook* classes are somewhat confusing to me, and I always have to rediscover how they relate. So I created an ASCII diagram of it. I think it's worth shoving in a comment somewhere, or in some "architecture" document. I'll use this issue as a placeholder for that.

                                                 ┌──────────┐
                                                 │ HookSpec │
                                                 └──────────┘
                                                       ▲       plugin1    ┌────────┐
                                                       │  ┌──────────────►│HookImpl│
                                                       │  │               └────────┘
                                       foo_hook1 ┌─────┴──┴─┐
                                      ┌──────────┤HookCaller├───►...
                                      │          └────────┬─┘
                                      │                   │               ┌────────┐
                                      │                   └──────────────►│HookImpl│
                                      │                        plugin2    └────────┘
    ┌─────────────┐  pm.hook  ┌───────┴─┐
    │PluginManager├──────────►│HookRelay├───►...
    └─────────────┘           └───────┬─┘
                                      │                        plugin2    ┌────────┐
                                      │                  ┌───────────────►│HookImpl│
                                      │                  │                └────────┘
                                      │          ┌───────┴──┐
                                      └─────────►│HookCaller├───►...
                                       foo_hook2 └─────┬─┬──┘
                                                       │ │                ┌────────┐
                                                       │ └───────────────►│HookImpl│
                                                       ▼       plugin3    └────────┘
                                                 ┌──────────┐
                                                 │ HookSpec │
                                                 └──────────┘

I was lucky to see this flowchart, which can be said to be a simple illustration of the core calling logic of pluggy. If you need to continue improving, or have something to help with, I'd be happy to do some!

nicoddemus commented 2 years ago

Gave it a quick go, just to see if it has potential:

flowchart LR
  PluginManager -.- pm.hook --> HookRelay
  HookRelay -- foo_hook --> HookCaller --> HookSpec
  HookRelay -- foo_hook2 --> HookCaller -- plugin1 --> HookImpl1
  HookCaller -- plugin2 --> HookImpl2

Looks nice! https://mermaid-js.github.io/mermaid

bluetech commented 2 years ago

Thanks for converting @nicoddemus!

It does lose some details - namely it merges the nodes when it's better not, the placing of the HookSpec and the .... Maybe this can be added in though. I'll need to learn the mermaid syntax.

CofinCup commented 1 year ago

[off topic] Is it possible that we use a class structure like this? Or are there specialised intentions? I mean, the HookRelay class seemed meaningless to me...

                               ┌──────────┐
                               │ HookSpec │
                               └──────────┘
                                     ▲       plugin1    ┌────────┐
                                     │  ┌──────────────►│HookImpl│
                                     │  │               └────────┘
                     pm.hook1  ┌─────┴──┴─┐
                    ┌──────────┤HookCaller├───►...
                    │          └────────┬─┘
                    │                   │               ┌────────┐
                    │                   └──────────────►│HookImpl│
                    │                        plugin2    └────────┘
┌─────────────┐     |
│PluginManager|-----|
└─────────────┘     |
                    │                        plugin2    ┌────────┐
                    │                  ┌───────────────►│HookImpl│
                    │                  │                └────────┘
                    │          ┌───────┴──┐
                    └─────────►│HookCaller├───►...
                     pm.hook2  └─────┬─┬──┘
                                     │ │                ┌────────┐
                                     │ └───────────────►│HookImpl│
                                     ▼       plugin3    └────────┘
                               ┌──────────┐
                               │ HookSpec │
                               └──────────┘
bluetech commented 11 months ago

@CofinCup HookRelay does seem redundant but I think the main problem it solves is that hook names don't conflict with PluginManager method/attribute names.