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.

Sign up for GitHub

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

Jump to bottom

voting: fetch active account votes in script #806

Merged
merged 6 commits into from
Apr 17, 2019
Merged

voting: fetch active account votes in script #806

merged 6 commits into from
Apr 17, 2019

Conversation

2color
Copy link
Contributor

@2color 2color commented Apr 16, 2019

What

  • Move active account vote fetching to worker script

Why

  • Previous, every update to votes from the app reducer would fetch voter state for all votes,
  • With this change, we only fetch all the votes when the active account changes and fetch individual votes' state when a CastVote eth event is processed

@2color 2color requested review from bpierre and sohkai April 16, 2019 20:55
sohkai
sohkai approved these changes Apr 16, 2019
View reviewed changes
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looking good, a couple of small clean up suggestions

apps/voting/app/src/vote-types.js Show resolved Hide resolved
apps/voting/app/src/script.js Outdated Show resolved Hide resolved
apps/voting/app/src/script.js Outdated Show resolved Hide resolved
apps/voting/app/src/script.js Outdated Show resolved Hide resolved
apps/voting/app/src/script.js Outdated Show resolved Hide resolved
apps/voting/app/src/script.js Show resolved Hide resolved
apps/voting/app/src/script.js Outdated Show resolved Hide resolved
@2color
Copy link
Contributor Author

2color commented Apr 16, 2019

Tested again after applying changes. Looking good

@2color 2color merged commit ca4c86b into master Apr 17, 2019
@2color 2color deleted the improve-vote-state branch April 17, 2019 08:27
ramilexe pushed a commit to ConsiderItDone/aragon-apps that referenced this pull request Dec 9, 2021
@2color
voting: fetch active account votes in script (aragon#806)
9272c33 
* voting: fetch active account votes in script

* Move connectedAccount out of the store and cache

* Add comment about using strings over symbols

* Move functions to helpers

* Update comments

Co-Authored-By: 2color <[email protected]>

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

@sohkai sohkai sohkai approved these changes

@bpierre bpierre Awaiting requested review from bpierre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@2color @sohkai