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

Update services diagrams #251

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Update services diagrams #251

merged 1 commit into from
Nov 26, 2024

Conversation

flomonster
Copy link
Contributor

No description provided.

@flomonster flomonster marked this pull request as draft November 19, 2024 16:29
@ElysaSrc
Copy link
Member

Overall looks good to me, is there any reason to not use mermaid ?

@flomonster flomonster force-pushed the fam/update-services-diagrams branch from cf47e15 to 305cbdb Compare November 19, 2024 17:13
@flomonster flomonster marked this pull request as ready for review November 19, 2024 17:15
@flomonster
Copy link
Contributor Author

flomonster commented Nov 21, 2024

Overall looks good to me, is there any reason to not use mermaid ?

  • It was quicker to reuse the old diagram.
  • Overall, I find it prettier to make a svg with more control than a mermaid.

If you insist on using mermaid I can make the change.

@emersion
Copy link
Member

Should we describe how the simulation results are pushed back from core to editoast? IOW: messages flow in the reverse direction as well.

@SergeCroise
Copy link
Contributor

Overall looks good to me, is there any reason to not use mermaid ?

  • It was quicker to go back to the old diagram.
  • Overall, I find it prettier to make a svg with more control than a mermaid.

If you insist on using mermaid I can make the change.

mermaid sources are easier to review, isn't it?

@flomonster
Copy link
Contributor Author

flomonster commented Nov 22, 2024

mermaid sources are easier to review, isn't it?

It is but I tried to reproduce it with mermaid and it was unreadable.
The problem is that if we want to keep the images inside the nodes, we have to include html code in the mermaid script...

Note

My mermaid skills may be limited 😅

@flomonster flomonster force-pushed the fam/update-services-diagrams branch from 305cbdb to 08463be Compare November 22, 2024 14:28
@flomonster
Copy link
Contributor Author

Should we describe how the simulation results are pushed back from core to editoast? IOW: messages flow in the reverse direction as well.

Fixed

Copy link
Contributor

@Castavo Castavo left a comment

Choose a reason for hiding this comment

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

You wrote "statics files are bundle" instead of "bundled"

Signed-off-by: Florian Amsallem <[email protected]>
@flomonster flomonster force-pushed the fam/update-services-diagrams branch from 08463be to 9423007 Compare November 25, 2024 09:00
@flomonster flomonster requested a review from Castavo November 25, 2024 09:00
@Castavo Castavo removed their request for review November 25, 2024 13:54
@Castavo
Copy link
Contributor

Castavo commented Nov 25, 2024

I don't know our current architecture enough to approve this PR
Unlike @emersion or @ElysaSrc

@flomonster flomonster merged commit 6c7fc64 into master Nov 26, 2024
4 checks passed
@flomonster flomonster deleted the fam/update-services-diagrams branch November 26, 2024 08:43
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.

5 participants