-
Notifications
You must be signed in to change notification settings - Fork 324
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: add docs for gluon-mesh-vpn-wireguard #2351
Conversation
No, I didn't. I'll rebase. |
3c1c0c5
to
426949c
Compare
@NeoRaider Thanks. Updated. |
15d1743
to
a355765
Compare
I pushed another commit explaining the MTU selection for tunneldigger and wireguard. |
This eventually closes #2320. |
#2186 has been merged, so this needs a rebase now. |
7f2107d
to
7c95ab7
Compare
7c95ab7
to
c2c07cb
Compare
@NeoRaider Thanks, updated. |
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'm fine with the text as it is and appreciate the documentation of the MTU calculation, as well as the section about explicitly setting MSS.
It's a bit slangly, like the section around "glued together" technologies, but reads well otherwise.
I'd like you to take a look at the capitalization of abbreviations like VLAN
or VXLAN
and whether you think it would disturb the reading flow.
The other suggestions are mainly WireGuard
as name,
mesh VPN
without the dash,
DS-Lite
with dash like in rfc 6333,
and minor spellfixes.
Last nitpick from me: could batman-adv
be referenced as such instead of batman
?
Otherwise this already is a great documentation, I'd say.
Thanks for the comments. Updated. |
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.
Looks nice, thanks.
Please squash the spelling fix commits into the commits they fix. |
9825278
to
b6dad53
Compare
@NeoRaider updated. |
b6dad53
to
3e8dc30
Compare
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.
This looks good. Both content and squash wise.
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 finally found the time to give this a more thorough look. My apologies for not doing this before asking you to squash your commits.
@NeoRaider Everything you requested has been taken into account. |
|
Works now. Please merge. |
There's a new merge conflict due to the merge of #2352. |
@NeoRaider Thanks for the hint. Rebased. |
Approved without explcitly using GitHub tools
No description provided.