-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
TeamTopologies diagram #4678
base: develop
Are you sure you want to change the base?
TeamTopologies diagram #4678
Conversation
@@ -0,0 +1,85 @@ | |||
import { log } from '../../logger.js'; |
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.
Please provide feedback on this DB file as I consider it "complete" at the moment.
@@ -0,0 +1,12 @@ | |||
export const draw = (txt, id, _version, diagObj) => { |
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.
Work on the renderer has not yet started.
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.
Hi there @Incognito, thanks for you contribution.
If the PR is WIP it would be better to mark it as draft.
And I wouldn't recommend using tt
as keyword is the best choice, the entity relationship diagram has the er
keyword, but I guess it's well-known shortcut.
Thanks @Yokozuna59 , I'll change it to |
@Incognito I was referring to mark it as draft, not renaming the title :) Draft: Ready to be merged: The keyword should inline with other keywords:
Not really sure what other possible rules we have, those what came to my mind right now. |
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.
Hi @Incognito, happy to see a new diagram type addition.
As @Yokozuna59 noted, the keyword should be teamTopology
.
Also, the grammar should include the keyword.
teamTopology
f1#Stream
f2#Stream
f1--XaaS->f2
I think the grammar requires some more features to support the features mentioned in the website.
We can continue the grammar discussion in the issue thread.
let teams = {}; | ||
let interactions = {}; |
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.
Use proper types.
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.
You'll have to add Interfaces and use them here.
mermaidAPI.parseDirective(this, statement, context, type); | ||
}; | ||
|
||
const addInteraction = function (fromTeam, interactionType, toTeam) { |
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.
Please make sure all functions are typed.
ExternalDiagramDefinition, | ||
} from '../../diagram-api/types.js'; | ||
|
||
const id = 'tt'; |
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.
teamTopology-beta
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.
Only this one spot or should beta suffix cascade into other parts of the code?
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.
Can you please add this to the new diagram documentation too?
packages/mermaid/src/diagrams/team-topology/teamTopologyDiagram.ts
Outdated
Show resolved
Hide resolved
…m.ts Co-authored-by: Sidharth Vinod <sidharthv96@gmail.com>
Acted on most feedback above. Topics still outstanding:
Any feedback on other topics is open, and if there's support from anyone to help more along any of the above I'm more than happy to collaborate with anyone spontaneously (I can only push this topic along in limited bursts of free time). |
FYI @manupaisable @matthewskelton if you are interested to take a look / cross-link it in your resources later. |
@Incognito do you have plans to continue on this PR? |
@mrueg Thanks for checking in. Yes I'd love to, my main constraint is really just my time however. When things slow down a bit I have space to push this, but the last few months have been pretty busy on my end. Happy to close it for now if having it open is distracting from the project's focus. I can re-open once it is ready for another round of review. Or we can keep it open. Your call :) |
@Incognito Understood, no need to close it from my perspective. I just wanted to check if that's something you still plan to work on (otherwise I'd try to take a look at when I get time) |
📑 Summary
This PR adds a Team Topologies diagram as per #4659 .
Resolves #4659
📏 Design Decisions
The grammar accepts a list of teams and team interactions to identify nodes and relationships. That list is later parsed and turned into a layout as per conventions from Team Topologies
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSION
is used for all new features.develop
branch