search

mingrammer / diagrams

:art: Diagram as Code for prototyping cloud system architectures
https://diagrams.mingrammer.com
MIT License
38.98k stars 2.51k forks source link

Type name appearing C4 nodes #892

Open jpgoldberg opened 1 year ago

jpgoldberg commented 1 year ago

There is a mismatch in the documented behavior for C4 and the actual behavior.

The image, ./img/c4.png, has the nodes produced without the "type" being prepended to the the optional technology argument.

Here is one of the nodes from the documentation (cropped from what is in the documentation

C4 as documented API Node

And here is the same node when actually running the sample code (cropped from my run of the sample code):

C4-generated-APInode

The difference is the text "Container: ". This difference appears in all nodes in the generated image.

I prefer the documented behavior, but either the behavior needs to generate what is documented or the documentation needs to be updated to match what is generated.

jpgoldberg commented 1 year ago

The line of code that adds the type to the text displayed there is in the definition of C4Node in diagrams/c4/__init__.py

    key = f"{type}: {technology}" if technology else type

Git is telling me that this was last modified by @mbruggmann in resolving issue #508. I haven't looked at that issue or PR to learn whether this behavior was added then and whether it was intentional.

mbruggmann commented 1 year ago

Hi @jpgoldberg 👋🏻

From what I remember I took the screenshot while prototyping, but then added this behavior to align with the recommendations from https://c4model.com/ (which do have the <TYPE>: prefix). Don't have a strong opinion on how it should work 🤷🏻

But you are right, we should at least make sure the documentation matches the behavior of the code.