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

Fix #239: Refactor route resources loading flow. #241

Merged
merged 7 commits into from
Sep 9, 2016

Conversation

n1k0
Copy link
Contributor

@n1k0 n1k0 commented Sep 8, 2016

No description provided.

@n1k0 n1k0 force-pushed the 239-route-loading-refactoring branch from eb1ed76 to e1908d3 Compare September 9, 2016 09:54
@n1k0 n1k0 merged commit 5539d1d into master Sep 9, 2016
@n1k0 n1k0 deleted the 239-route-loading-refactoring branch September 9, 2016 11:44
@glasserc
Copy link
Contributor

glasserc commented Sep 9, 2016

Sometimes it's hard for me to review this kind of PR because I don't really understand it. I'm going to ask some questions to try to get a better sense of what's happening. These aren't critiques, just an attempt to follow along with what you're doing so that I can review stuff in the future if I need to.

What is a "route resource"? It seems like a remote resource, meaning either a bucket, a group, a collection, or a record. Is that correct?

What is being refactored about it? It seems like you're pulling together these resources and DRYing out some logic about deciding whether you are busy or not. Is that correct?

@n1k0
Copy link
Contributor Author

n1k0 commented Sep 9, 2016

What is a "route resource"? It seems like a remote resource, meaning either a bucket, a group, a collection, or a record. Is that correct?

Yes, basically a unique server resource mapped to a URL parameter. We fetch these automatically on each route change in a batch HTTP request.

What is being refactored about it? It seems like you're pulling together these resources and DRYing out some logic about deciding whether you are busy or not. Is that correct?

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants