redux-logger icon indicating copy to clipboard operation
redux-logger copied to clipboard

Refactor index.js & add more tests

Open mfeniseycopes opened this issue 9 years ago • 7 comments

Refactors index.js to utilize more helper methods and make code more semantic. No changes in logic or naming conventions, just shifting things around. Also, moves early returns to the top of createLogger to prevent unnecessary assignments and variable creation.

Also, adds more specs to index.spec.js (still needs more though) and moves helper tests to helpers.spec.js.

Let me know what you think!

mfeniseycopes avatar Apr 16 '17 14:04 mfeniseycopes

Codecov Report

Merging #223 into master will decrease coverage by 14.21%. The diff coverage is 89.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #223       +/-   ##
===========================================
- Coverage   83.56%   69.34%   -14.22%     
===========================================
  Files           5        5               
  Lines         146      137        -9     
===========================================
- Hits          122       95       -27     
- Misses         24       42       +18
Impacted Files Coverage Δ
src/index.js 89.74% <89.28%> (+2.9%) :arrow_up:
src/diff.js 8% <0%> (-92%) :arrow_down:
src/core.js 78.68% <0%> (+2.63%) :arrow_up:

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 07cb031...ace19b8. Read the comment docs.

codecov-io avatar Apr 16 '17 14:04 codecov-io

Sorry for delay, will review on this week.

imevro avatar Apr 19 '17 18:04 imevro

But it looks pretty good!

imevro avatar Apr 19 '17 18:04 imevro

Can you please fix merge conflicts?

imevro avatar May 17 '17 11:05 imevro

Renamed #noLogger to #hasLogger and rebased to resolve merge conflicts.

mfeniseycopes avatar May 19 '17 12:05 mfeniseycopes

@evgenyrodionov Have you had a chance to look this over?

mfeniseycopes avatar Jun 30 '17 15:06 mfeniseycopes

@mfeniseycopes yes, after #234. Sorry for a delay, too busy now.

imevro avatar Jun 30 '17 15:06 imevro