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.

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

doc: changed order of invocations in example. #9614

Closed
wants to merge 1 commit into from

Conversation

atripes
Copy link

@atripes atripes commented Nov 15, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. https Issues or PRs related to the https subsystem. labels Nov 15, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Nov 15, 2016

I'm not getting an error prior to this change.

@jasnell
Copy link
Member

jasnell commented Nov 18, 2016

Not getting an error either. That said, I prefer this ordering.

@cjihrig
Copy link
Contributor

cjihrig commented Nov 18, 2016

Yea, I do prefer this order too.

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM with commit message fixed as I also don't get an error with current order.


req.on('error', (e) => {
console.error(e);
});
req.end();

Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this blank line.

italoacasas pushed a commit that referenced this pull request Nov 24, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@italoacasas
Copy link
Contributor

Landed in 550a958

Thanks for the contribution @atrioom

@lpinca
Copy link
Member

lpinca commented Nov 24, 2016

@italoacasas the landed commit does not follow the commit message guidelines.

italoacasas pushed a commit that referenced this pull request Nov 24, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@italoacasas
Copy link
Contributor

@lpinca I made some changes, I hope that one fit better.

@lpinca
Copy link
Member

lpinca commented Nov 24, 2016

@italoacasas not a biggie but the second line ("When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.") is too long. Those lines should be wrapped at 72 chars.
Title has a final dot.

@atripes
Copy link
Author

atripes commented Nov 25, 2016

Hello and thank you for the help. I had this issue in a full grown application, I will try to reproduce it in a small test environment. Maybe add modules until I get the error. Thanks for accepting it anyways.

addaleax pushed a commit that referenced this pull request Dec 5, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.

PR-URL: #9614
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This was referenced Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. https Issues or PRs related to the https subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants