Skip to content

Conversation

hiroppy
Copy link
Member

@hiroppy hiroppy commented Jan 14, 2017

Verify all cases.
Place: https://github.com/nodejs/node/blob/master/lib/_http_incoming.js#L136
Coverage: https://coverage.nodejs.org/coverage-57f6a106fbc69a47/root/_http_incoming.js.html

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. lts-watch-v6.x labels Jan 14, 2017
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Jan 14, 2017
@hiroppy hiroppy force-pushed the feature/create-http-incoming-matchKnownFields.js branch from e94e7a9 to 3392d40 Compare January 15, 2017 06:56
Copy link
Member

Choose a reason for hiding this comment

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

You could simplify this to:

const dest = {};
if (value)
  dest[field] = 'test';

Copy link
Member

Choose a reason for hiding this comment

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

Typo: s/Incomingmessage/IncomingMessage

@santigimeno
Copy link
Member

@hiroppy hiroppy force-pushed the feature/create-http-incoming-matchKnownFields.js branch from 3392d40 to fe0f0c2 Compare January 15, 2017 09:00
Copy link
Member

@jasnell jasnell 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 green CI

@jasnell
Copy link
Member

jasnell commented Jan 16, 2017

Landed in dd629ba

@jasnell jasnell closed this Jan 16, 2017
@hiroppy hiroppy deleted the feature/create-http-incoming-matchKnownFields.js branch January 17, 2017 13:50
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
@MylesBorins
Copy link
Contributor

This test is failing on v4.x

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

Labels

http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants