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

DOCS(dev): Convert network protocol rst to md #6715

Merged
merged 17 commits into from
Jan 27, 2025

Conversation

Kissaki
Copy link
Member

@Kissaki Kissaki commented Jan 26, 2025

Convert our network protocol docs from rst (ReStructuredText) to md (Markdown) format.


This changeset is a followup to PR #6623. In that PR, which first integrated these docs into this repository, the comments indicated consensus that the docs should be in Markdown format to match our other docs.

This changeset is based on PR #6714, which does some baseline corrections, and which should be merged first.


Two tables are not converted to Markdown tables because they are not flat tables, which is hard, if not impossible, to represent in Markdown. Preformatted code blocks are being used for them instead.

Otherwise, the conversion seems straightforward.

The changeset includes individual commits by change type, so each kind of change can be traced. The first commit is purely a rename to ensure Git follows the content history across the file conversion. Unless there is interest in the individual changes, a review of the target state or full diff makes the most sense/is enough.


Checks

The cross-document reference apparently worked on readthedocs (our previous docs), but does not work on GitHub.

The cross-document figure reference is replaced with a file reference.
The figure follows below the paragraph.
This is done as a separate commit to retain Git change file history association. (Git does not store renames. It assumes renames according to content similarity.)
Two tables are not converted because they are not flat tables but contain "complex" content/structure.
@Kissaki
Copy link
Member Author

Kissaki commented Jan 26, 2025

I initially tried converting with pandoc, but that did not result in complete, correct, clean Markdown. So I did these conversions manually, step-by-step.

Copy link
Member

@Krzmbrzl Krzmbrzl left a comment

Choose a reason for hiding this comment

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

I would vote for linking to the actual Protobuf file for checking the exact structure for messages. Duplicating their structure here only adds potential for making them become out of sync.

Also, I do realize that my review comments have nothing to do with the conversion to Markdown itself. So I'd be fine if we address them in another follow-up PR (though for me it'd also be fine to do it here).

| `os_version` | `string` |

The version field is a combination of major, minor and patch version numbers (e.g. 1.2.0)
so that major number takes two bytes and minor and patch numbers take one byte each.
Copy link
Member

Choose a reason for hiding this comment

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

Iirc this is now the legacy version format. We now use two bytes for every part.
I would tend to just mention the new format but would be fine with mentioning both.

features such as encryption are implemented by the transport layer. Integers
above 8-bits are encoded using the *Variable length integer encoding*.

## Packet format
Copy link
Member

Choose a reason for hiding this comment

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

Imo we should just describe the new Protobuf based format.

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not familiar with a change of old and new formats.

I see MumbleUDP.proto. So this doc reported an older voice channel protocol?

Do current clients support legacy and new, and servers default to proto if available on the client?

@Kissaki
Copy link
Member Author

Kissaki commented Jan 27, 2025

I'll address them as follow-ups to keep this MR/changeset narrow/specific scope.

@Kissaki Kissaki merged commit 9e30471 into mumble-voip:master Jan 27, 2025
17 checks passed
@Kissaki Kissaki deleted the docs/netwprot3 branch January 27, 2025 17:01
@Kissaki
Copy link
Member Author

Kissaki commented Jan 27, 2025

I'm currently drafting a first changeset to update references, improve text, and add notes about outdated. I'm considering doing a more thorough fixup/cleanup/removal afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants