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

Create mermaid diagram for FRAME section #280

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dawnkelly09
Copy link
Collaborator

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.

@dawnkelly09 dawnkelly09 requested a review from a team as a code owner December 19, 2024 17:53
Copy link
Collaborator

@CrackTheCode016 CrackTheCode016 left a 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.

@nhussein11
Copy link
Collaborator

@nhussein11 can you improve this one as well? I'm not sure how I feel about this current version.

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

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 :)

@nhussein11 nhussein11 requested a review from kianenigma January 3, 2025 14:06
subgraph sg2[Runtime]
direction LR
G[Runtime API]
H[Executive]
Copy link
Contributor

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:

Copy link
Contributor

@kianenigma kianenigma left a 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!

@eshaben eshaben added B0 - Needs Review Pull request is ready for review C0 - Low Low priority task A0 - New Content Pull request contains new content pages labels Jan 13, 2025
@dawnkelly09 dawnkelly09 requested a review from a team as a code owner January 15, 2025 15:06
@dawnkelly09
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 - New Content Pull request contains new content pages B0 - Needs Review Pull request is ready for review C0 - Low Low priority task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants