-
Notifications
You must be signed in to change notification settings - Fork 6
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
More descriptive name for HandleEth() #30
Comments
Note, the Put term comes from Go's encoding/binary package terminology which refers to writing data of length L into a buffer that needs to be at least of length L. See
|
I like PutNextOutboundPacket - that would be a huge improvement
…On Fri, 19 Jul 2024 at 09:49, Patricio Whittingslow < ***@***.***> wrote:
Note, the *Put* term comes from Go's encoding/binary package terminology
which refers to writing data of length L into a buffer that needs to be at
least of length L. See
More ideas
- PutNextOutboundPacket
- PutOutbound
- PutNextPacket
—
Reply to this email directly, view it on GitHub
<#30 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALR2VA3P5IPYMR557FSJW2LZNDHLJAVCNFSM6AAAAABK43BHCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZYGY4DOMBWGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
So it occured to me that to better match with |
I think the 'next' in the word is descriptive - it helps me imagine how it
works .. and the 'packet' again .. descriptive and useful - the Eth.. sure
if it's specifically Ethernet - add it
which would take us to PutNextOutboundEthPacket .. which might appear
comically long, but honestly I don't think that's a bad thing (for
something so fundamental)
HandleEth appears a lot in comments too .. so 'rename symbol' isn't going
to get all of them
…On Sat, 20 Jul 2024 at 13:20, Patricio Whittingslow < ***@***.***> wrote:
So it occured to me that to better match with RecvEth maybe we could do
PutOutboundEth, what do you think? Packet is pretty generic and Eth makes
it clear it refers to link layer packets.
—
Reply to this email directly, view it on GitHub
<#30 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALR2VA6SXDGXWJQTDC3WN5TZNJIYFAVCNFSM6AAAAABK43BHCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRGEYTKMZZHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I agree the naming should be something between This interface should be the first thing developers and users study when beginning working with any Go link-layer API. And I tend to disagree on the point of fundamental things needing be more explicit. In any case it being so fundamental should be more reason for the name to be short and memorable since it will be something outward facing and well known. We don't do I also feel giving it a long name would not be consistent with the naming we have given |
It is your baby Patricio
And I don’t have the context or experience you do
My fundamental point really was always that it should be clear that it is
sending, and ideally, what it is sending
Happy to go with whatever you decide
…On Sat, 20 Jul 2024 at 15:49, Patricio Whittingslow < ***@***.***> wrote:
I agree the naming should be something between PutOutboundEth and
PutNextOutboundEthPacket. I tend to err on the side of shortness since
this will form part of a "famous" and fundamental interface:
netif.InterfaceEthPoller
<https://github.com/soypat/netif/blob/e7dfef0020a669328a65683e3e6115b4b7426af0/netif.go#L34>
.
This interface should be the first thing developers and users study when
beginning working with any Go link-layer API. And I tend to disagree on the
point of fundamental things needing be more explicit. In any case it being
so fundamental should be more reason for the name to be short and memorable
since it will be something outward facing and well known. We don't do Vector.AddVector(v
Vector), in Go we prefer Vector.Add(v Vector). This is what gonum
<https://github.com/gonum/gonum/blob/0c62273e338b91cd9578ed93572c693ba55e1eaa/spatial/r3/vector.go#L15>
does, and that's a project I have huge respect for in terms of API design
I also feel giving it a long name would not be consistent with the naming
we have given RecvEth. Even though RecvEth has a short name and omits the
Next and Packet words in the name (even though they'd serve to describe it
better) the purpose is quite clear- the Stack receives an Ethernet (packet!
what else would it receive!). There is no question about whether it is the
next or previous packet :)
—
Reply to this email directly, view it on GitHub
<#30 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALR2VAZ47IS7DD7WV7XUFKTZNJ2IVAVCNFSM6AAAAABK43BHCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRGE3TGNJRHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hey nick, I applied the changes in this commit: 74e7a6b Also centralized the documentation at the handlers to avoid duplicating comments. Should be easily navigable in VSCode since the |
Brainstorm:-
func (ps *PortStack) handleEth(dst []byte) (n int, err error) {
//might be better named as getNextOutboundPacket
or fillNextOutboundPacket
The text was updated successfully, but these errors were encountered: