Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Use strict' @top and updated the file with const/let [issue-702] #726

Merged
merged 2 commits into from Feb 5, 2019

Conversation

Projects
None yet
4 participants
@SahibArora
Copy link
Contributor

SahibArora commented Jan 30, 2019

Added strict mode in filer.spec.js
Learn more about strict > https://www.w3schools.com/js/js_strict.asp

var abs = "..";

// changed to 

const abs = "..";
// Variables which are objects of library are changed to const and rest are changed to let.

Please advice if I missed something.
Thank you

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 30, 2019

Codecov Report

Merging #726 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #726      +/-   ##
==========================================
- Coverage   86.71%   86.63%   -0.08%     
==========================================
  Files          16       16              
  Lines        1746     1736      -10     
==========================================
- Hits         1514     1504      -10     
  Misses        232      232
Impacted Files Coverage Δ
src/filesystem/interface.js 93.29% <0%> (-0.39%) ⬇️
src/fs-watcher.js 92.3% <0%> (ø) ⬆️
src/shell/environment.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26b47ee...8bcbdbc. Read the comment docs.

@SahibArora SahibArora changed the title 'Use strict' @top and updated the file with const/let 'Use strict' @top and updated the file with const/let [issue-702] Jan 31, 2019

@@ -45,21 +46,21 @@ describe('Filer', function() {
});

it('must honor the \'FORMAT\' flag', function(done) {
var name = 'local-test';
let name = 'local-test';

This comment has been minimized.

@pbrahmbhatt3

pbrahmbhatt3 Jan 31, 2019

Contributor

Hi @SahibArora

Suggested change
let name = 'local-test';
const name = 'local-test';

I think you can update the name variable to be a const as it is not getting reassigned in the code.

This comment has been minimized.

@SahibArora

SahibArora Feb 1, 2019

Author Contributor

Hi @pbrahmbhatt3,
Thanks for coming and looking over.
I will do the necessary changes.

@SahibArora

This comment has been minimized.

Copy link
Contributor Author

SahibArora commented Feb 1, 2019

done!

@humphd

humphd approved these changes Feb 5, 2019

Copy link
Contributor

humphd left a comment

This looks great, nice work.

@humphd

This comment has been minimized.

Copy link
Contributor

humphd commented Feb 5, 2019

One tip @SahibArora: you use [issue-702] in your issue above, but it would be better to say Fixes #702, which GitHub will use to auto-close your bug when this PR lands. Next time!

@humphd humphd merged commit b5e1d9a into filerjs:master Feb 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.