Naming of NATSymmetricUDPFirewall

ccding / go-stun

A go implementation of the STUN client (RFC 3489 and RFC 5389)

Apache License 2.0

675 stars 120 forks source link

Naming of NATSymmetricUDPFirewall #33

Closed olifre closed 4 years ago

olifre commented 5 years ago

It took me a while to understand that NATSymmetricUDPFirewall actually means Symmetric UDP Firewall without any actual NAT being involved (as is also shown in the RFC and the flow chart in https://github.com/ccding/go-stun/blob/master/stun/discover.go ).

I wonder whether this constant should be renamed / aliased to SymmetricUDPFirewall to better clarify that this is very different from NATSymmetric ?

xiang90 commented 5 years ago

@olifre

can you send a PR to fix this?

olifre commented 5 years ago

@xiang90 In principle yes, but there are two problems:

I don't know if there should be an alias (for a while?) to not break API compatibilitiy with other users (e.g. syncthing) hard.
I basically only know Go well enough to read it, so I would only be able to do a search / replace (but well, that's really sufficient here I think :wink: ).

ccding commented 4 years ago

closing this since no PR. The name indicates the meaning: https://github.com/ccding/go-stun/blob/master/stun/const.go#L62

olifre commented 4 years ago

@ccding I could of course do a PR on this, but I'd love feedback first if an alias should be kept and if there's general agreement on a new name. The current name seems irritating even though the description is clear. Since the broken name NATSymmetricUDPFirewall has already lead to confusion in Syncthing (which did consider this a NAT, but it's completely unrelated), I don't think it's good to keep that.

ccding commented 4 years ago

@olifre Can you have a look at #34 ?