-
Notifications
You must be signed in to change notification settings - Fork 8
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
Create mermaid diagram for FRAME section #280
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall correct to me, @kianenigma would be the best one to ask here.
@dawnkelly09 I gave this one a shot. I like how it looks, but I feel that we need to add the following to help users understand that those pallets are not the only ones that can be added; they are just examples: graph TB
subgraph sg1[Client/Host]
direction LR
A[JSON-RPC]
B[libp2p]
C[Transaction Pool]
D[Block Builder]
E[Database]
F[Host Functions]
end
subgraph sg2[Runtime]
direction LR
G[Runtime API]
H[Executive]
I[frame_support::runtime]
subgraph sg3[Pallets]
direction TB
J[System]
K[Staking]
L[Balances]
M[GRANDPA]
N[...]
end
end
However, I do agree with @CrackTheCode016 and feel that @kianenigma's input would be valuable here to determine whether this diagram is accurate. Note: After getting Kian's input, I can circle back on this to see if the diagram requires any style modifications or not :) |
subgraph sg2[Runtime] | ||
direction LR | ||
G[Runtime API] | ||
H[Executive] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you explained "executive" anywhere? I worry this might be some low level of detail that the old docs had and we are mistakenly copying it.
For any new keyword added, ask yourself, does the reader already know what this is? if yes, good. If no, either remove, or add a link to where it is explained.
Executive is a fairly low level detail of FRAME.
I have some better resources for you to create a more informed version of this diagram:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do slightly better, let's iterate one more time based on the resources I provided, thanks!
@nhussein11 Updated this so it's the diagram showing the larger group of pallets and then the selected group to create a runtime. Definitely needs some styling if you can help with that. @0xLucca mentioned: "This runtime could be link to the solochain-template as it mimics the pallets included in that runtime" but, I'm not sure I can make the mermaid clickable? I added a "tip" admonition but feel free to change things if you know how to link from the diagram or think of a better way to share that info. Thank you for your help! |
This PR adds a first draft mermaid diagram for the FRAME section.
@nhussein11 can you improve this one as well? I'm not sure how I feel about this current version.