Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add content for hole punching #200
Add content for hole punching #200
Changes from 1 commit
cab291e
fe771ff
f0b0172
39b3ab0
b967be2
356b513
7b01c29
cb9b074
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Let's not use any of these Plantumls. Every time I see them, my eyes start to hurt.
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.
Hmm, how about we transform them into simpler diagrams that are direct? Here's an idea I sketched out (only showing packetA here and can of course be improved/corrected).
cc. @mxinden
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.
I don't have an opinion here.
Another option would be to use mermaid. GitHub supports it. I am not sure our CMS does.
https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-diagrams
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.
That could work too. As these diagrams already exist for hole punching, does it make sense to include them in this PR and then create an issue to plan out what we can substitute them for?
In general, I'd like to further enhance the concept docs with visuals and establish a guideline we can follow. As the docs seem to aim for high-level content and clean explainers, we may decide to have simple and fun graphics such as this, or perhaps more direct diagrams like the one above.
I also think it's good to have UML diagrams exist somewhere, as they can serve as a more formal representation.
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.
Would also add that we currently have similar sequence diagrams on some of the concept pages, circuit relay is an example. What do we think @marten-seemann @mxinden?
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 folks. We'll be looking to get this off the ground asap. In the meantime, does it make sense to keep the uml diagrams in this PR and substitute them once we have new ones?
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.
Yup, I think that's reasonable.
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.
@DannyS03 I guess a minor nit from me is: I'd rather commit the images to this repo (in a media/ folder) and link to it's location there. Imo it's better than hosting it on imgur
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.
Yes, likewise, I was just trying to limit the merge conflicts with the structure changes we make and planned to add them after
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.
#207 <- Tracking issue to update diagrams once we have them available