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

Class diagram #341

Open
bluetech opened this issue Jan 10, 2022 · 8 comments
Open

Class diagram #341

bluetech opened this issue Jan 10, 2022 · 8 comments
Assignees

Comments

@bluetech
Copy link
Member

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 │
                                                 └──────────┘
@bluetech bluetech self-assigned this Jan 10, 2022
@RonnyPfannschmidt
Copy link
Member

Maybe we should plntuml this

@bluetech
Copy link
Member Author

Just saw this github/roadmap#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
Copy link
Member

sounds fun, will wait for that then,

@Northxw
Copy link

Northxw commented Mar 10, 2022

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
Copy link
Member

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
Copy link
Member Author

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
Copy link

[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
Copy link
Member Author

bluetech commented Aug 5, 2023

@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.

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

5 participants