Skip to content
New issue

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

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

Already on GitHub? # to your account

Consolidated application state store. #1721

Merged
merged 5 commits into from
Aug 16, 2019
Merged

Consolidated application state store. #1721

merged 5 commits into from
Aug 16, 2019

Conversation

tonyanziano
Copy link
Contributor

@tonyanziano tonyanziano commented Aug 13, 2019

Resolves #1697

===

This is a huge PR, so here are a couple of notes:

  • The real magic happens in forwardToMain.ts, forwardToRenderer.ts, & store.ts on both main and client
  • The reducers & actions had to be combined and duplicated on both sides. I'm thinking that in a future PR, these could all be moved to the /app/shared/ package so that we only need one copy of each reducer and set of actions
  • Removed a lot of unnecessary code calling between main / renderer to sync data
  • Removed some unused / unnecessary reducer & action code
  • Added missing tests for all state actions (planning to add tests for all state reducers) DONE

@cwhitten
Copy link
Member

@tonyanziano from the description it looks like you still plan to implement some additional tests?

(planning to add tests for all state reducers)

@tonyanziano
Copy link
Contributor Author

@cwhitten Correct, but I was thinking of including them in a separate PR to keep this one as small as possible.

I can include them in this PR though if you'd prefer.

@coveralls
Copy link

coveralls commented Aug 15, 2019

Coverage Status

Coverage increased (+2.4%) to 65.323% when pulling 452a118 on toanzian/state into 54e8cbe on master.

@tonyanziano
Copy link
Contributor Author

Reducer tests have been added.

@tonyanziano tonyanziano merged commit 889b7d2 into master Aug 16, 2019
@tonyanziano tonyanziano deleted the toanzian/state branch August 16, 2019 20:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consolidated state management
3 participants