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

Spamming of 'setSubscription' #1303

Closed
faboweb opened this issue Sep 11, 2018 · 9 comments · Fixed by #1616
Closed

Spamming of 'setSubscription' #1303

faboweb opened this issue Sep 11, 2018 · 9 comments · Fixed by #1616
Assignees
Labels
bug 🐛 issues related to unhandled errors in the code that need to be fixed low priority has been discussed, will be addressed later

Comments

@faboweb
Copy link
Collaborator

faboweb commented Sep 11, 2018

UI Version: 0.10.3

Description:

image

The commit 'setSubscription' is called continuously.

@faboweb faboweb added bug 🐛 issues related to unhandled errors in the code that need to be fixed discuss labels Sep 11, 2018
@fedekunze fedekunze added low priority has been discussed, will be addressed later and removed discuss labels Sep 18, 2018
@NodeGuy NodeGuy removed high priority ❗ low priority has been discussed, will be addressed later labels Oct 30, 2018
@NodeGuy
Copy link
Contributor

NodeGuy commented Nov 5, 2018

I can't reproduce this.

@jbibla
Copy link
Collaborator

jbibla commented Nov 6, 2018

i have definitely seen this (in the vue console)

@faboweb faboweb added the low priority has been discussed, will be addressed later label Nov 6, 2018
@faboweb
Copy link
Collaborator Author

faboweb commented Nov 6, 2018

Low priority as this doesn't effect the user

@NodeGuy
Copy link
Contributor

NodeGuy commented Nov 7, 2018

@faboweb After I prioritized a lot of bugs you said we don't prioritize bugs so I removed all of the labels. Do you want to start prioritizing bugs?

@jbibla Can you reproduce it?

@faboweb
Copy link
Collaborator Author

faboweb commented Nov 7, 2018

I reproduced this issue.
We said we would always do bugs right away but for situations when we decide not do them as it is not important to do them. In this case the issue doesn't effect the user, so I thought we might as well not do the bug now.

@NodeGuy
Copy link
Contributor

NodeGuy commented Nov 8, 2018

I reproduced this issue.

How did you reproduce the bug? This information will be useful (necessary) for the developer assigned to fixing it.

We said we would always do bugs right away but for situations when we decide not do them as it is not important to do them. In this case the issue doesn't effect the user, so I thought we might as well not do the bug now.

So are you saying that non-user facing bugs can be low priority and all user-facing bugs are implicitly high priority?

@faboweb
Copy link
Collaborator Author

faboweb commented Nov 11, 2018

So are you saying that non-user facing bugs can be low priority and all user-facing bugs are implicitly high priority?

Sounds like a good sum up. Should we add this t our handbook?

How did you reproduce the bug?

Like Jordan already mentioned: Open Voyager > Open Vue-Dev-Tools ( > maybe sign in) wait a couple of seconds.

@NodeGuy
Copy link
Contributor

NodeGuy commented Nov 12, 2018

Sounds like a good sum up. Should we add this t our handbook?

I think it would be helpful because I assigned low priority to a bunch of bugs after reading the following in the Handbook:

We decide on the priority of the issue by adding a label “high priority” or “low priority”. In doubt add the “high priority” label. We focus on those and can switch the label label.

@faboweb
Copy link
Collaborator Author

faboweb commented Nov 20, 2018

This is called in blockchain.js on every block to show that the subscription is active. We just need to not send to commit if state.subscription = true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 issues related to unhandled errors in the code that need to be fixed low priority has been discussed, will be addressed later
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants