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

Improved store persist. #3424

Merged
merged 2 commits into from Nov 17, 2017

Conversation

Projects
None yet
2 participants
@jorgefilipecosta
Member

jorgefilipecosta commented Nov 10, 2017

This PR comes from #3331, and includes the possible improves we found to store-persist during the discussion.

Store persist is now generic with no access to default values.
Store persist receives the default values of the specific reducer key passed to it.
Receives an options object instead of a set of parameters.

Show outdated Hide outdated editor/store.js Outdated
@gziolo

This comment has been minimized.

Show comment
Hide comment
@gziolo

gziolo Nov 10, 2017

Member

I haven't followed the discussion, but those changes look good to me. It looks like a nice improvement 👍

Member

gziolo commented Nov 10, 2017

I haven't followed the discussion, but those changes look good to me. It looks like a nice improvement 👍

Improved store persist.
Store persist is now generic with no access to default values.
Store persist receives the default values of the specific reducer key passed to it.
Receives an options object instead of a set of parameters.
@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Nov 16, 2017

Codecov Report

Merging #3424 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3424   +/-   ##
=======================================
  Coverage   34.54%   34.54%           
=======================================
  Files         261      261           
  Lines        6710     6710           
  Branches     1225     1225           
=======================================
  Hits         2318     2318           
  Misses       3704     3704           
  Partials      688      688
Impacted Files Coverage Δ
editor/store.js 83.33% <ø> (ø) ⬆️
editor/store-persist.js 100% <ø> (ø) ⬆️
editor/reducer.js 90.27% <ø> (ø) ⬆️
editor/store-defaults.js 100% <100%> (ø) ⬆️

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 38d0c74...ac9d8c1. Read the comment docs.

codecov bot commented Nov 16, 2017

Codecov Report

Merging #3424 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3424   +/-   ##
=======================================
  Coverage   34.54%   34.54%           
=======================================
  Files         261      261           
  Lines        6710     6710           
  Branches     1225     1225           
=======================================
  Hits         2318     2318           
  Misses       3704     3704           
  Partials      688      688
Impacted Files Coverage Δ
editor/store.js 83.33% <ø> (ø) ⬆️
editor/store-persist.js 100% <ø> (ø) ⬆️
editor/reducer.js 90.27% <ø> (ø) ⬆️
editor/store-defaults.js 100% <100%> (ø) ⬆️

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 38d0c74...ac9d8c1. Read the comment docs.

Changed store defaults to export an object with preferences instead o…
…f an object with single key named preferences.
@jorgefilipecosta

This comment has been minimized.

Show comment
Hide comment
@jorgefilipecosta

jorgefilipecosta Nov 16, 2017

Member

Hi @gziolo I did some changes that try to make the improvement you suggested to the defaults, feel free to have a new look and let me know if I did not understand correctly your suggestion.

Member

jorgefilipecosta commented Nov 16, 2017

Hi @gziolo I did some changes that try to make the improvement you suggested to the defaults, feel free to have a new look and let me know if I did not understand correctly your suggestion.

@gziolo

gziolo approved these changes Nov 17, 2017

It looks great, small but nice refactoring 🚢

@jorgefilipecosta jorgefilipecosta merged commit 5c93d78 into master Nov 17, 2017

3 checks passed

codecov/project 34.54% (+0%) compared to 38d0c74
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jorgefilipecosta jorgefilipecosta deleted the update/improved-store-persist branch Nov 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment