Skip to content
This repository has been archived by the owner on Feb 26, 2021. It is now read-only.

Instead of storing todo.userId, use publish composite to control th… #49

Merged
merged 2 commits into from
Dec 9, 2015

Conversation

tmeasday
Copy link
Contributor

…e publication.

@stubailo
Copy link
Contributor

We need to update the method tests, since they check that the denormalization worked.

@tmeasday
Copy link
Contributor Author

Yup, was going to do that, just wanted to check with you that it still made sense given this comment: #48 (comment)

@stubailo
Copy link
Contributor

Oh that's interesting. That actually might be a good reason to keep the denorm... we could probably get it to 0 lookups.

@tmeasday
Copy link
Contributor Author

I'm not sure what the priority here is. There are a few competing things:

  1. Achieving best performance (seems misguided? but we do it in other places)
  2. Demonstrating the most patterns for the guide
  3. Being a clear and simple example app

Actually all three things compete with each other. I'm not sure what's best. Let's discuss tomorrow.

@stubailo
Copy link
Contributor

I think we should go with "4: Write what we would do in a real app" - and I think I'd probably do the publish-composite thing until it became a real performance issue. So I think we should take the PR.

@tmeasday
Copy link
Contributor Author

This is ready to merge, I'll let you hit the button and double check @stubailo

@stubailo
Copy link
Contributor

stubailo commented Dec 9, 2015

Looks great - I rebased on master

stubailo pushed a commit that referenced this pull request Dec 9, 2015
Instead of storing `todo.userId`, use publish composite to control th…
@stubailo stubailo merged commit 67cb595 into master Dec 9, 2015
@tmeasday tmeasday deleted the no-todo-userId branch March 7, 2016 02:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants