Skip to content

Conversation

flub
Copy link
Contributor

@flub flub commented Aug 26, 2025

I haven't given this a read-through myself yes, so this could still be
rough. Review for:

  • Consistency of the story.
  • Amount of detail given, too much, too little?
  • Consistency of tone. It's a bit of a different tone from the last
    post I wrote. I didn't feel like using overusing the voice from the
    last post.

And of course, spelling grammar, sentence structure. There are some
rather too long sentences still I think, I'll still try to improve
those in a next round.

Copy link

vercel bot commented Aug 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
iroh-computer Ready Ready Preview Comment Aug 29, 2025 9:13am

@flub
Copy link
Contributor Author

flub commented Aug 26, 2025

Erm, it's not showing up in the preview. But comparing to my last post I think I do the same things. What did I do wrong?

@matheus23
Copy link
Member

There we go

@flub
Copy link
Contributor Author

flub commented Aug 26, 2025

Skimming this now I wonder if the whole NAT Types section needs to be there. I added it to try and explain why you need to contact multiple STUN or QAD servers, because STUN has an obscure feature where a single STUN server can bind to multiple IP addresses and you can ask the STUN server to contact you from a different address to check if you're endpoint-dependent or endpoint-independent and QAD does not have an equivalent mechanism so it seems like a fair thing to put into a STUN vs QAD comparison. But... that was a whole load of details that I didn't end up writing, because in the end I thought it was too niche.

@n0bot n0bot bot added this to iroh Aug 26, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Aug 26, 2025
@matheus23
Copy link
Member

FWIW I liked the NAT types section. I wish more people adopted the endpoint-dependent and endpoint-independent terms, and sprinkling in some NAT teachings into this blog post makes sense IMO.

Copy link
Member

@ramfox ramfox left a comment

Choose a reason for hiding this comment

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

Ayy, this is great & very informative, though I fear I may have crushed your voice in this review 😂

But I think that the stun bits might be 30% too sassy

Otherwise, I mostly read for obvious grammar mistakes. Will do another pass!

@flub
Copy link
Contributor Author

flub commented Aug 27, 2025

Ayy, this is great & very informative, though I fear I may have crushed your voice in this review 😂

No worries, that's exactly the feedback I asked for!

@flub
Copy link
Contributor Author

flub commented Aug 27, 2025

I've gone through the entire thing again. Thanks @matheus23 for the feedback on the NAT Types section being useful, I've left it in as it is already referred to by the surrounding bits. I think this might be in a reasonable shape now.

@flub flub requested a review from ramfox August 27, 2025 10:58
I don't mind other titles either, but this is already an improvement
Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Typos!

Co-authored-by: Philipp Krüger <[email protected]>
Copy link
Member

@ramfox ramfox left a comment

Choose a reason for hiding this comment

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

Only went through half of this so far, I have to run to help out a friend for an hour, but I'll be back to finish soon!

Copy link
Member

@ramfox ramfox left a comment

Choose a reason for hiding this comment

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

Looks great!

Made a last batch of suggested changes.

Just make sure to adjust the date and then we should merge/publish 🙌

there was a need for endpoints to learn about their reflexive transport addresses.
For this the STUN spec was created,
which by now has evolved into [RFC 8489].
A sizable tome.
Copy link
Member

Choose a reason for hiding this comment

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

😂

export const post = {
draft: false,
author: 'Floris Bruynooghe',
date: '2025-08-26',
Copy link
Member

Choose a reason for hiding this comment

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

if we want to publish this for monday, we should just put 2025-09-01 & then merge. The post will then just self publish on monday.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does still show up in the preview, does the preview work differently?

@flub flub merged commit e33b931 into main Sep 2, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Sep 2, 2025
@flub flub deleted the flub/qad branch September 2, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants