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

TeamTopologies diagram #4678

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Incognito
Copy link
Contributor

📑 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

@@ -0,0 +1,85 @@
import { log } from '../../logger.js';
Copy link
Contributor Author

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) => {
Copy link
Contributor Author

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.

Copy link
Member

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

@Incognito Incognito changed the title [WIP] TeamTopologies diagrams [draft] TeamTopologies diagrams Jul 29, 2023
@Incognito
Copy link
Contributor Author

Thanks @Yokozuna59 , I'll change it to team-topology or something more expressive.

@Yokozuna59 Yokozuna59 marked this pull request as draft July 29, 2023 10:25
@Yokozuna59
Copy link
Member

@Incognito I was referring to mark it as draft, not renaming the title :)

Draft:

BCF9793C-1763-4CE5-ACCE-983B6F9BBB87

Ready to be merged:

ADFD1596-C424-40B4-B627-57EE0021409A


The keyword should inline with other keywords:

  1. single word (if possible).
  2. unique name (shouldn't interfere with other existing diagrams).
  3. lowercase (if two words then camelCase.
  4. shouldn't have anything not specific to the diagram (trailing Diagram or Chart, i.e, classDiagram)
  5. If there is a possible layout difference, add an option after the diagram keyword, i.e., flowchart TB (TB means top to bottom layout).

Not really sure what other possible rules we have, those what came to my mind right now.

Copy link
Member

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

Comment on lines +15 to +16
let teams = {};
let interactions = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use proper types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mermaidAPI.parseDirective(this, statement, context, type);
};

const addInteraction = function (fromTeam, interactionType, toTeam) {
Copy link
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

teamTopology-beta

Copy link
Contributor Author

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4459

Can you please add this to the new diagram documentation too?

@Incognito
Copy link
Contributor Author

Incognito commented Aug 1, 2023

Acted on most feedback above.

Topics still outstanding:

  1. Adding types
  2. Support for quotes, team names with whitepsace
  3. Unicode team names, markdown team names
  4. Diagram renderer
  5. Diagram docs

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

@mrueg
Copy link

mrueg commented Aug 1, 2023

FYI @manupaisable @matthewskelton if you are interested to take a look / cross-link it in your resources later.

@sidharthv96 sidharthv96 changed the title [draft] TeamTopologies diagrams TeamTopologies diagram Aug 2, 2023
@mrueg
Copy link

mrueg commented Jan 15, 2024

@Incognito do you have plans to continue on this PR?

@Incognito
Copy link
Contributor Author

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

@mrueg
Copy link

mrueg commented Jan 25, 2024

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

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

Successfully merging this pull request may close these issues.

Diagram Proposal: Team Topologies
4 participants