-
Notifications
You must be signed in to change notification settings - Fork 0
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
WIP: isLoading: set true earlier, and false later #103
base: master
Are you sure you want to change the base?
Conversation
isLoading() = false | ||
} | ||
case Failure(_) => | ||
//TODO: send empty graph event? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when getNewGraph fails, I think we should clear the graph here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//TODO: send empty graph event? | |
eventProcessor.localEvents.onNext(ReplaceGraph(Graph.empty)).foreach { _ => | |
isLoading() = false | |
} |
} | ||
case Failure(_) => | ||
//TODO: send empty graph event? | ||
isLoading() = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLoading() = false |
I think, this generally is probably "more correct", but also a bit error-prone because you have to reset isLoading in multiple places at the end. It is a bit difficult to read on the github web ui. Best thing would probably issuing a new-increasing sequence number for each request. then let rawgraph return the sequence number it belongs to. Then isLoading just becomes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know.
No description provided.