Addressed Issue #703 #711
Addressed Issue #703 #711
Conversation
Codecov Report
@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 86.71% 86.71% +<.01%
==========================================
Files 16 16
Lines 1746 1739 -7
==========================================
- Hits 1514 1508 -6
+ Misses 232 231 -1
Continue to review full report at Codecov.
|
|
Some variables that were declared with let could be declared with const but this is more stylistic preference. Nice job! 💯 |
| @@ -1,15 +1,15 @@ | |||
| var util = require('../lib/test-utils.js'); | |||
| var expect = require('chai').expect; | |||
| const util = require('../lib/test-utils.js'); | |||
There was a problem hiding this comment.
Add 'use strict'; at the top of your file to use strict mode.
There was a problem hiding this comment.
This is a good start. There are some things we can do to improve it:
- Please follow-up on the feedback from @ithompson4 re:
use strict - Let's revert the unrelated change to
package-lock.json: usegit checkout master package-lock.jsonto get the correct version in this PR. - You can change all your
letuses below toconstwhen the variable doesn't change in the given scope--I think that's true of all of them. You can try replacing them all, and run the tests. It will flag any that are wrong.
Please make these changes and let me know when it's ready for re-review.
|
@dcoull how is this coming? |
Changes made as requested |
|
@dcoull this still didn't work, maybe we should try this together when we see each other tomorrow. Can you bring your laptop and we do it together? |
Updated variable declarations in times.spec.js from var to let and const. All tests passed