Skip to content

Add back semicolons to the HTTP server example in about page - #656

Merged
fhemberger merged 1 commit into
masterfrom
revert-641-semicolonless
Apr 18, 2016
Merged

Add back semicolons to the HTTP server example in about page#656
fhemberger merged 1 commit into
masterfrom
revert-641-semicolonless

Conversation

@lpinca

Copy link
Copy Markdown
Member

Reverts #641

Not sure what are the better practices there, I guess the purpose was to test the bot.
Anyway I'm filing this pr because semicolons have been removed and this makes the example inconsistent with all the others (guides, API docs, etc).

cc: @Fishrock123

@Fishrock123

Fishrock123 commented Apr 17, 2016

Copy link
Copy Markdown
Contributor

Feel free to re-add the semicolons but the rest is actually better practice, I.e. Not using writeHead() directly

@lpinca

lpinca commented Apr 17, 2016

Copy link
Copy Markdown
MemberAuthor

Forgive my ignorance but what's wrong with writeHead()? It's a public api afaik.

@Fishrock123

Copy link
Copy Markdown
Contributor

Nothing is wrong with it, but chances are you are not going to be calling it directly from user code. :)

@lpinca
lpincaforce-pushed the revert-641-semicolonless branch from 1192126 to 2d664b2CompareApril 18, 2016 06:43
@lpincalpinca changed the title Revert "About: update example to use better practices"Add back semicolons to the HTTP server example in about pageApr 18, 2016
@lpinca

Copy link
Copy Markdown
MemberAuthor

I'm still not sure why I should prefer

res.statusCode=200;res.setHeader('Content-Type','text/plain');

over

res.writeHead(200,{'Content-Type': 'text/plain'});

but it doesn't matter. I've updated the pr to only re-add the semicolons.

@fhemberger
fhemberger merged commit a0cd761 into masterApr 18, 2016
@fhemberger
fhemberger deleted the revert-641-semicolonless branch April 18, 2016 16:42
@fhemberger

Copy link
Copy Markdown
Contributor

Cool, thanks!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@lpinca@Fishrock123@fhemberger