[Harsh review] Add print-numbers.js - #1
Conversation
kendaleiv
left a comment
kendaleiv
left a comment
There was a problem hiding this comment.
Please address my comments.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
There was a problem hiding this comment.
There are no tests for this method. Please add them.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
| for (var i = 1; i <= 10; i++) { | |||
| console.log(i); | |||
There was a problem hiding this comment.
You should use a logging library, rather than logging with console.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
| for (var i = 1; i <= 10; i++) { | |||
There was a problem hiding this comment.
This shouldn't only count to 10, it should accept the number to count to as a parameter.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
| for (var i = 1; i <= 10; i++) { | |||
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
There was a problem hiding this comment.
This function isn't invoked.
| for (var i = 1; i <= 10; i++) { | ||
| console.log(i); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This file doesn't end with a newline.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
There was a problem hiding this comment.
There's no documentation for how to use this.
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
There was a problem hiding this comment.
We use 2 spaces for indentation, not 4.
|
Add a PR description, it's blank. Amend your commit to include the same text. |
| @@ -0,0 +1,5 @@ | |||
| function printNumbers() { | |||
There was a problem hiding this comment.
Rename to displayNumbers. Rename the file. Update the PR title.
1 participant
No description provided.