FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Adjust `test-http-content-length` and docs to consider countdown by Bamieh · Pull Request #17201 · nodejs/node · GitHub

/ node Public

Adjust test-http-content-length and docs to consider countdown - #17201

Closed
Bamieh wants to merge 3 commits into
nodejs:masterfrom
Bamieh:refactor-test-http-content-length-and-docs
ConversationCommits (3)ChecksFiles changed
Closed

Adjust test-http-content-length and docs to consider countdown#17201
Bamieh wants to merge 3 commits into
nodejs:masterfrom
Bamieh:refactor-test-http-content-length-and-docs

Conversation

Bamieh commented Nov 21, 2017

Copy link
Copy Markdown
Contributor

Refactored the test suite test-http-content-length to use countdown and referred to the countdown usage in the docs; as per issue #17169

P.S. These small refactors are really good for new contributors to get more personal with the code base. @jasnell I am willing to contribute more to these refactors if needed.

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

doc, test

nodejs-github-bot added doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Nov 21, 2017
jasnell reviewed Nov 21, 2017
View reviewed changes
Comment thread doc/guides/writing-tests.md Outdated

jasnell Nov 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

just a nit: can you line wrap at <= 80 chars :-)

Bamieh Nov 21, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@jasnell Sure 😄

mscdex added the http Issues or PRs related to the http subsystem. label Nov 21, 2017

Trott commented Nov 22, 2017

Copy link
Copy Markdown
Member

Should this be two separate commits? One for the doc and one for the test?

Bamieh commented Nov 22, 2017

Copy link
Copy Markdown
Contributor Author

@Trott the changes are minor so i figured its okay to put them together. i can split them if thats a requirement.

maclover7 commented Nov 24, 2017

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/11695/

addaleax commented Nov 30, 2017

Copy link
Copy Markdown
Member

@Bamieh Yeah, splitting them would be ideal, but generally that can also be done by whoever lands this

Bamieh commented Dec 1, 2017

Copy link
Copy Markdown
Contributor Author

@addaleax I will definitely do this in future pull requests, but should i split them right now to save time for the person who's gonna land them?

maclover7 commented Dec 4, 2017

Copy link
Copy Markdown
Contributor

@Bamieh Would you be able to split the commits? After that I think this should be ready to land :)

maclover7 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 4, 2017
Bamieh force-pushed the refactor-test-http-content-length-and-docs branch from def969d to fef3cf6 Compare December 8, 2017 13:56

Bamieh commented Dec 8, 2017

Copy link
Copy Markdown
Contributor Author

@maclover7 I merged the commits into two, one for the fix, another for the documentation:

maclover7 commented Dec 8, 2017

Copy link
Copy Markdown
Contributor

@Bamieh it looks like there's still doc edits in the first commit -- are you able to move those to the second commit?

apapirovski removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 9, 2017
maclover7 added the wip Issues and PRs that are still a work in progress. label Dec 12, 2017

maclover7 commented Dec 12, 2017

Copy link
Copy Markdown
Contributor

Ping @Bamieh

Bamieh commented Dec 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@maclover7 sorry i got distracted. ready now.

Bamieh force-pushed the refactor-test-http-content-length-and-docs branch from fef3cf6 to f758516 Compare December 12, 2017 20:31
Bamieh force-pushed the refactor-test-http-content-length-and-docs branch from f758516 to 879f939 Compare December 12, 2017 20:44

maclover7 commented Dec 14, 2017

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/12109/

maclover7 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed wip Issues and PRs that are still a work in progress. labels Dec 14, 2017
maclover7 pushed a commit that referenced this pull request Dec 17, 2017
doc: add countdown module to writing tests guide
cf76176 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
maclover7 pushed a commit that referenced this pull request Dec 17, 2017
test: update test-http-content-length to use countdown
97cee72 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
doc: add countdown module to writing tests guide
92d08d9 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
test: update test-http-content-length to use countdown
c626f8a 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
doc: add countdown module to writing tests guide
d1af106 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
test: update test-http-content-length to use countdown
46f8a9e 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins mentioned this pull request Jan 10, 2018
Merged
boingoing pushed a commit to nodejs/node-chakracore that referenced this pull request Jan 18, 2018
doc: add countdown module to writing tests guide
9b4495e 
PR-URL: nodejs/node#17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
boingoing pushed a commit to nodejs/node-chakracore that referenced this pull request Jan 18, 2018
test: update test-http-content-length to use countdown
a783c4f 
PR-URL: nodejs/node#17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2018
doc: add countdown module to writing tests guide
cd88918 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2018
test: update test-http-content-length to use countdown
005e367 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2018
doc: add countdown module to writing tests guide
d56b589 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2018
test: update test-http-content-length to use countdown
ec1e260 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
gibfahn pushed a commit that referenced this pull request Jan 24, 2018
doc: add countdown module to writing tests guide
2f35920 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
gibfahn pushed a commit that referenced this pull request Jan 24, 2018
test: update test-http-content-length to use countdown
6ed5773 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins mentioned this pull request Jan 24, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
doc: add countdown module to writing tests guide
fa393d5 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
test: update test-http-content-length to use countdown
97f764c 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
doc: add countdown module to writing tests guide
505280f 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
test: update test-http-content-length to use countdown
cb16791 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
doc: add countdown module to writing tests guide
d0b89a1 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
test: update test-http-content-length to use countdown
31c5db6 
PR-URL: #17201
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
gireeshpunathil gireeshpunathil approved these changes
+2 more reviewers
evanlucas evanlucas approved these changes
maclover7 maclover7 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

maclover7

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL