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

v2: Collection#parse #3664

Merged
merged 1 commit into from
Jun 4, 2015
Merged

Conversation

jridgewell
Copy link
Collaborator

This is the revert of #3659 (which reverted #3644). So, really, it's just #3644 (which reverted #1407... reverts! 😛).

Why? To mirror Model. Model#set doesn’t parse for you, only instantiation, #fetch, and #save parse.

  • We do it on instantiation because we have to have an instance to call #parse (you can't do it before hand because you don't have an instance)
  • We do it on #fetch and #save because they return data from the server (the reason we have a #parse!)

If you need to parse your Collection's data before #set, #reset, or #add, do it beforehand (you already have an instance):

  • collection.set(collection.parse(models));
  • collection.reset(collection.parse(models));
  • collection.add(collection.parse(models));

Or, you can parse on instantiation if you don't have an instance, just like Model:

  • new Collection(models, { parse: true });

And of course, Collection#fetch parses at the collection level like it always has.

…parse-v1-2-1"

This reverts commit 3e334b1, reversing
changes made to ff57b54.
@jridgewell jridgewell changed the title Collection#parse v2 v2: Collection#parse Jun 4, 2015
@akre54
Copy link
Collaborator

akre54 commented Jun 4, 2015

So are we good? We done reverting?

@akre54 akre54 added the fixed label Jun 4, 2015
akre54 added a commit that referenced this pull request Jun 4, 2015
@akre54 akre54 merged commit a91e8ea into jashkenas:master Jun 4, 2015
@jridgewell
Copy link
Collaborator Author

Our patch release hasn't shipped yet.... 😢.

@jridgewell
Copy link
Collaborator Author

Instead of reverting again, mind if I just rebase it off of master? I promise not to submit another breaking PR until v1.2.1 ships.

@akre54
Copy link
Collaborator

akre54 commented Jun 4, 2015

Not sure what you mean...

@jridgewell
Copy link
Collaborator Author

I made #3659 so we didn't ship a (major) breaking change in a patch release. #3644 (comment)
This is the revert of that PR meant to be released in the next major.

@akre54
Copy link
Collaborator

akre54 commented Jun 4, 2015

Ok right but keep in mind that you should never rebase something that others are potentially using. I doubt anyone has pulled from master in the last 2 minutes, but we really should never modify the history of master or bad things will happen.

(if in the future something shouldn't be merged, please put a big DO NOT MERGE tag on it!)

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

Successfully merging this pull request may close these issues.

2 participants