-
Notifications
You must be signed in to change notification settings - Fork 142
Update architecture diagram to use mermaid #2788
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
base: master
Are you sure you want to change the base?
Conversation
Replaces original .pptx source file and the generated .png
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2788 +/- ##
=======================================
Coverage 71.67% 71.67%
=======================================
Files 134 134
Lines 7284 7284
Branches 1514 1516 +2
=======================================
Hits 5221 5221
Misses 1936 1936
Partials 127 127 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Normally, the PowerPoint option gives us a lot more control which we can use to make the diagram easier to understand using direction/colour/shape cues. On the other hand, the mermaid/puml version is easier to update incrementally. In some cases the cost is worth the convenience. |
|
I think we can go ahead with using mermaid, but the diagram still needs some refinement. I played around with the mermaid diagram to make it look more compact, and I think we should make sure that the new diagram at least
|
gerteck
left a comment
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.
Thanks for working on this! Requesting some changes to make the graph look more compact and understandable at a glance!
|
|
||
|  | ||
| <mermaid> | ||
| graph TD |
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.
Instead of TD (top down), use LR (left right)
| classDef title fill:#6fa8dc,stroke:none,color:white,font-weight:bold; | ||
|
|
||
| %% --- 1. Top Level Structure --- | ||
| CLI[index.js #40;cli#41;] |
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.
Consider grouping these together
E.g.
subgraph Core [Core Site Structure]
direction LR
Site[Site Object]
Page[Page]
end
| end | ||
|
|
||
| %% --- 4. The Legend (Optional Hack to display legend) --- | ||
| subgraph LegendBox [Legend] |
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.
no need legend if we can inline the meanings



Replaces original .pptx source file and the generated .png
What is the purpose of this pull request?
Overview of changes:
Change replaced architecture diagram form
architecture.pptxandarchitecture.pngto use mermaiddiagram integrated in
architecture.mdAnything you'd like to highlight/discuss:
Testing instructions:
markbind servein/docs/devGuide/design/architecture.mdProposed commit message: (wrap lines at 72 characters)
Update architecture diagram to use mermaid
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):