remove _toString, always use NAME_KEY #14485
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@krisselden r? |
stefanpenner
force-pushed
the
name-key
branch
4 times, most recently
from
abdefe5
to
d269b8d
Compare
rwjblue
reviewed
Oct 17, 2016
| @@ -1,6 +1,6 @@ | |||
| import { dictionary, symbol, setOwner, OWNER } from 'ember-utils'; | |||
| import { ENV } from 'ember-environment'; | |||
| import { assert, deprecate, runInDebug } from 'ember-metal'; | |||
| import { assert, deprecate, runInDebug, NAME_KEY } from 'ember-metal'; | |||
There was a problem hiding this comment.
Can you move NAME_KEY (which I think is just a symbol right?) into the ember-utils package instead? We are ultimately trying to make container package not rely on ember-metal (the assert, deprecate, runInDebug functionality was the only remaining bit).
There was a problem hiding this comment.
i made the change, but something after moving that stuff around has broken things ... need to figure out what
stefanpenner
force-pushed
the
name-key
branch
3 times, most recently
from
951beff
to
b65e1a8
Compare
no longer: * have two concepts * processAllNamespaces + find NAME_KEY if the name was pre-determined by the container (previously _toString was set, but not NAME_KEY, which made this code assume it must calculate the name again) * change shape of `this` with a constant descriptor of toString * have _toString (NAME_KEY is now used)
stefanpenner
force-pushed
the
name-key
branch
from
b65e1a8
to
f8f259a
Compare
|
This looks good to me... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
no longer:
thiswith a constant descriptor of toStringI noticed ember-data's test suite was spending an unexpected amount of time in processAllNamespaces, which lead me to realize
_toStringis not consider in classToString, but it really should have been.