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

Fix #625 Add test for fs.readlink() when the path isn't a symbolic link #744

merged 1 commit into from Feb 26, 2019


Copy link

commented Feb 26, 2019

Fix for issue #625

Added a test for fs.readlink() when the path isn't a symbolic link.


This comment has been minimized.

Copy link

commented Feb 26, 2019

Codecov Report

Merging #744 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #744      +/-   ##
+ Coverage    86.6%   86.65%   +0.05%     
  Files          16       16              
  Lines        1739     1739              
+ Hits         1506     1507       +1     
+ Misses        233      232       -1
Impacted Files Coverage Δ
src/filesystem/implementation.js 83.77% <0%> (+0.09%) ⬆️

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 2a4fa0f...e3da133. Read the comment docs.

humphd approved these changes Feb 26, 2019
Copy link

left a comment

This does exactly what we needed, and I can see the coverage info has gone up. Nice work.

One thing to be aware of: if you look at the changes this PR makes, it's touching a lot of lines that aren't related to the code you're adding. It looks like your editor has automatically added extra whitespace in lots of places. This is something you'll want to avoid when submitting PRs to projects, since altering these lines will create what is known as "spurious whitespace changes." In other words, it looks like these lines were altered as part of this commit, but in reality, only the whitespace was changed. Doing this makes it harder to track the history of code back in time.

I'll allow it here, since it's not a huge deal in this case. But be aware for the future, and alter your editor to not do this, or you'll have your PRs rejected.

@humphd humphd merged commit 914ba8b into filerjs:master Feb 26, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
3 participants
You can’t perform that action at this time.