chore: fix some comments #644
Conversation
Signed-off-by: drawdrop <[email protected]>
src/udp_packets.rs
Outdated
| @@ -14,7 +14,7 @@ pub mod udp_payload { | |||
| /// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | |||
| /// | Length | Checksum | | |||
| /// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | |||
| /// We append the data, based on the port the the header | |||
| /// We append the data, based on the port the header | |||
There was a problem hiding this comment.
Although I agree with the change, does this sentence make sense?
"…based on the port the header"
There was a problem hiding this comment.
Ah, should this be "the port in the header"?
There was a problem hiding this comment.
"We append the data, based on the port"
There was a problem hiding this comment.
As in that should be the complete sentence?
I think that makes it a bit ambiguous: it's below a diagram with both "Source Port" and "Destination Port"—to which one is it referring? Or neither?
I think it'd be useful to clarify that as part of this, if possible…?
There was a problem hiding this comment.
Ahh then it should be "We append the data, based on the destination port"
There was a problem hiding this comment.
Perfect—@drawdrop, would you be okay making that minor change?
Otherwise, LGTM.
There was a problem hiding this comment.
Scratch that: I didn't see the merge conflict.
@drawdrop, if you can fix the merge conflict, this should be good to merge.
|
Fixed conflits myself :) |
No description provided.