Skip to content
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.

Sign up for GitHub

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

Jump to bottom

doc: fixup socket.remoteAddress #4198

Closed
wants to merge 1 commit into from
Closed

doc: fixup socket.remoteAddress #4198

wants to merge 1 commit into from

Conversation

baloo
Copy link
Contributor

@baloo baloo commented Dec 8, 2015

remoteAddress may be undefined under certain conditions

Trott
Trott reviewed Dec 8, 2015
View reviewed changes
doc/api/net.markdown
@@ -413,7 +413,8 @@ Returns `socket`.
### socket.remoteAddress

The string representation of the remote IP address. For example,
`'74.125.127.100'` or `'2001:4860:a005::68'`.
`'74.125.127.100'` or `'2001:4860:a005::68'`. Value may be `undefined` if
socket is destroyed (client disconnected, ...).
Copy link
Member

Choose a reason for hiding this comment

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

The parenthetical seems confusing more than clarifying. I would say either remove it, or change it to be more explicit: (for example, if the client disconnected)

Copy link
Member

Choose a reason for hiding this comment

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

Nit: if socket is -> if the socket is

@baloo
doc: fixup socket.remoteAddress
0e2359a 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>
@baloo
Copy link
Contributor Author

baloo commented Dec 8, 2015

@Trott fixed thanks

@mscdex mscdex added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Dec 8, 2015
@evanlucas
Copy link
Contributor

LGTM

baloo pushed a commit to baloo/forwarded that referenced this pull request Dec 9, 2015
test: remoteAddress may be undefined
fece5e2 
see nodejs/node#4198

test case:
``` javascript
var net = require('net');
var server = net.createServer(function(c) { //'connection' listener
  c.on('end', function() {
    console.log(c.remoteAddress);
    server.close();
  });
});
server.listen(8124, function() {
  var client = net.connect({port: 8124}, function(c) {
    client.end();
  });
});
```

Signed-off-by: Arthur Gautier <[email protected]>
@baloo baloo mentioned this pull request Dec 9, 2015
Closed
baloo pushed a commit to baloo/forwarded that referenced this pull request Dec 9, 2015
test: remoteAddress may be undefined
b4a0f71 
see nodejs/node#4198

test case:
``` javascript
var net = require('net');
var server = net.createServer(function(c) { //'connection' listener
  c.on('end', function() {
    console.log(c.remoteAddress);
    server.close();
  });
});
server.listen(8124, function() {
  var client = net.connect({port: 8124}, function(c) {
    client.end();
  });
});
```

Signed-off-by: Arthur Gautier <[email protected]>
@baloo baloo mentioned this pull request Dec 10, 2015
Closed
@jasnell
Copy link
Member

jasnell commented Dec 11, 2015

LGTM

rvagg pushed a commit that referenced this pull request Dec 14, 2015
@baloo @rvagg
doc: fixup socket.remoteAddress
671347c 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>

PR-URL: #4198
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@rvagg
Copy link
Member

rvagg commented Dec 14, 2015

LGTM, landed @ 671347c

Looks like this is your first contribution to core @baloo, thanks for the PR and welcome on board!

@rvagg rvagg closed this Dec 14, 2015
@Fishrock123 Fishrock123 mentioned this pull request Dec 15, 2015
Closed
cjihrig pushed a commit that referenced this pull request Dec 15, 2015
@baloo @cjihrig
doc: fixup socket.remoteAddress
5168472 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>

PR-URL: #4198
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@rvagg rvagg mentioned this pull request Dec 17, 2015
Closed
MylesBorins pushed a commit that referenced this pull request Dec 29, 2015
@baloo
doc: fixup socket.remoteAddress
aa1034a 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>

PR-URL: #4198
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
@baloo
doc: fixup socket.remoteAddress
b71a3b3 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>

PR-URL: #4198
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
Merged
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
@baloo
doc: fixup socket.remoteAddress
bb20511 
remoteAddress may be undefined under certain conditions

Signed-off-by: Arthur Gautier <[email protected]>

PR-URL: nodejs#4198
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@baloo @evanlucas @jasnell @rvagg @Trott @mscdex @MylesBorins