GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: fix confusing reference in net.md by vsemozhetbyt · Pull Request #12247 · nodejs/node

Conversation

@vsemozhetbyt

@vsemozhetbyt vsemozhetbyt commented Apr 5, 2017

Copy link
Copy Markdown
Contributor
Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, net

The mentioned echo server is described not previously but in a section below.

I am not sure about rewording, please correct me if needed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to Node.js documentation. net Issues and PRs related to the net subsystem. labels Apr 5, 2017
Trott
Trott approved these changes Apr 5, 2017
View reviewed changes
jasnell
jasnell requested changes Apr 6, 2017
View reviewed changes
Comment thread doc/api/net.md Outdated

@jasnell jasnell Apr 6, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Perhaps...

Following is an example...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vsemozhetbyt vsemozhetbyt Apr 6, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@jasnell Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vsemozhetbyt
doc: fix confusing reference in net.md
b9f9df9
cjihrig
cjihrig approved these changes Apr 6, 2017
View reviewed changes
sam-github
sam-github suggested changes Apr 6, 2017
View reviewed changes
Comment thread doc/api/net.md
it starts the connection.

Here is an example of a client of the previously described echo server:
Following is an example of a client of the echo server described

@sam-github sam-github Apr 6, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Following -> Here (I'm not sure your text is grammatical, I think it would have to be "Following this is an ...", but just "Here" was fine.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vsemozhetbyt vsemozhetbyt Apr 6, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@sam-github This is a wording proposed by @jasnell above. Now I am not sure what to choose)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sam-github
sam-github approved these changes Apr 6, 2017
View reviewed changes

@sam-github sam-github left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

OK, sure, it works, lets just fix it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
lpinca
lpinca approved these changes Apr 7, 2017
View reviewed changes
@aqrln

aqrln commented Apr 7, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@jasnell ping. Does this LGTY after @vsemozhetbyt has updated the PR?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Apr 7, 2017

Copy link
Copy Markdown
Member

Yes, lgtm!!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aqrln
aqrln approved these changes Apr 7, 2017
View reviewed changes
@vsemozhetbyt vsemozhetbyt added lts-watch-v4.x labels Apr 8, 2017
@vsemozhetbyt

vsemozhetbyt commented Apr 8, 2017

Copy link
Copy Markdown
Contributor Author

Landed in 14749f9
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vsemozhetbyt vsemozhetbyt closed this Apr 8, 2017
@vsemozhetbyt
vsemozhetbyt deleted the net.md branch April 8, 2017 13:12
vsemozhetbyt added a commit that referenced this pull request Apr 8, 2017
@vsemozhetbyt
doc: fix confusing reference in net.md
14749f9 
PR-URL: #12247
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Apr 10, 2017
@vsemozhetbyt Italo A. Casas
doc: fix confusing reference in net.md
4a6bb37 
PR-URL: nodejs#12247
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@italoacasas italoacasas mentioned this pull request Apr 10, 2017
v7.9.0 Release Proposal #12319
Merged
2 tasks done
MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
@vsemozhetbyt @MylesBorins
doc: fix confusing reference in net.md
6577f3e 
PR-URL: #12247
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@MylesBorins MylesBorins added dont-land-on-v4.x and removed lts-watch-v4.x labels Apr 18, 2017
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
@vsemozhetbyt @MylesBorins
doc: fix confusing reference in net.md
34f9dfd 
PR-URL: #12247
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 19, 2017
v6.10.3 proposal #12498
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@vsemozhetbyt @andrew749
doc: fix confusing reference in net.md
7b2a4bf 
PR-URL: nodejs/node#12247
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Alexey Orlenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell requested changes
@Trott Trott Trott approved these changes
@lpinca lpinca lpinca approved these changes
@cjihrig cjihrig cjihrig approved these changes
+2 more reviewers
@sam-github sam-github sam-github approved these changes
@aqrln aqrln aqrln approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation. 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.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@vsemozhetbyt @aqrln @jasnell @sam-github @Trott @lpinca @cjihrig @MylesBorins @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.