doc: Update rinfo object definition for UDP message event #10050
Conversation
nodejs-github-bot
added
dgram
imyller
added
the
code-and-learn
|
/cc @nodejs/documentation |
| `msg` argument is a [`Buffer`][] and `rinfo` is an object with the sender's | ||
| address information provided by the `address`, `family` and `port` properties: | ||
| * `address` {String} The sender's address | ||
| * `family` {String} The sender's family |
There was a problem hiding this comment.
"The address's protocol family", perhaps? The address family is same for sender and receiver, ipv4 or ipv6, but this text makes it sound like its sender specific. Also, what is the value? Is it a string 'ipv4'/6? Something else?
| console.log('Received %d bytes from %s:%d\n', | ||
| msg.length, rinfo.address, rinfo.port); | ||
| console.log('Received %d bytes from %s:%d using %s with a message size of %d\n', | ||
| msg.length, rinfo.address, rinfo.port, rinfo.family, rinfo.size); | ||
| }); |
There was a problem hiding this comment.
Would be nice to show some example output in a comment afterwards, to justify the existence of the example. As-is, I think this so-called "example" offers exactly zero additional benefit over the docs, and would be better deleted. People already know how to write functions in js, and attach event listeners, which is all this shows.
|
Ping @mcrummey: Any chance you can update this in accordance with the comments from @sam-github? |
mcrummey
force-pushed
the
document-rinfo-object
branch
from
e3c4d01
to
a46c873
Compare
|
Updated the PR to include @sam-github comments. Thanks for your patience and please let me know there are any other issues. |
|
@sam-github @nodejs/documentation Is this an OK way to show the output of the sample code? If so, anyone want to approve this change? If not, anyone want to suggest a better way to do it to @mcrummey? |
| `msg` argument is a [`Buffer`][] and `rinfo` is an object with the sender's | ||
| address information provided by the `address`, `family` and `port` properties: | ||
| * `address` {String} The sender's address | ||
| * `family` {String} The address's family |
There was a problem hiding this comment.
should say what the address family string can be, 'IPv4' or 'IPv6', I think? Its not easy to guess. The example below shows the IPv4 output, but in some places in the API various protocol type identifiers are not handled uniformly
|
Looks like the standard way in our docs to show example code output. I still feel the example offers no value. The only thing it shows that the docs don't is what the address family string can be for IPv4, and for that purpose, it is incomplete, because what about IPv6? Does anybody object to deleting the example? |
I guess I wouldn't object, but I also feel like sample code is inherently valuable. |
|
Even a one line example that shows a console.log of an object? What is the value? |
Since you seem to feel it should be removed, and I could go either way on it, let's remove it (unless someone else has a different opinion). |
mcrummey
force-pushed
the
document-rinfo-object
branch
from
a46c873
to
c757255
Compare
Detailing fields of object
|
Added IPv4|IPv6 and removed example. |
Provide details for fields of rinfo object of UDP message event. PR-URL: nodejs#10050 Reviewed-By: James M Snell <jasnell@gmail.com>
|
An aside: We could use greater standardization for these elements of an API. For example, which of the below is preferred?
...and so on. It seems like there's a lot of permutations in the docs. @nodejs/documentation |
|
I like
personally, I think dashes are normally used for giving more info about something. Sounds like we should have a I think the current method of "it looks like this is the style that was used two lines up, so let's stick with that" is a little haphazard. |
|
Heh, I dislike dashes there, but will accept nearly anything if it means we will have a single consistent style. :-D I think I'd prefer no punctuation, especially given how the docs render on the website:
That dash looks out of place to me, like a mistake. If there must be punctuation, a colon makes more sense to me than a dash. It also has the added benefit of eliminating the hyphen vs. en dash vs. em dash decision/confusion. But... if everyone loves a dash/hyphen/whatever there, then I'll get on board! No problem! :-D |
|
I agree it looks a bit weird on the website, I was looking at the Github markdown formatting:
which I think works quite well. I agree the dash looks too short on the website, and that we don't really want to get in to the en/em dash differences. I think on the website you're right, no punctuation will work next to that big green EDIT: To clarify, I think it's more important that it looks good on the website, as that's where most people will be looking at it. So +1 for nothing. |
|
Ah, I see, yeah, the dash looks just fine on GitHub itself. |
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
|
Can someone confirm that these changes apply to v6 and v4? |
|
@MylesBorins I confirm that they apply back to v4 (and probably much farther). |
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
Provide details for fields of rinfo object of UDP message event. PR-URL: #10050 Reviewed-By: James M Snell <jasnell@gmail.com>
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
doc
Description of change
Detailing fields of object