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.

Sign up for GitHub

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

Jump to bottom

See about cleaning up OrderedSet #15041

Closed
locks opened this issue Mar 20, 2017 · 6 comments
Closed

See about cleaning up OrderedSet #15041

locks opened this issue Mar 20, 2017 · 6 comments
Assignees
@locks

Comments

@locks
Copy link
Contributor

locks commented Mar 20, 2017 •
edited by Serabe
Loading

Source: https://github.com/emberjs/ember.js/blob/v2.11.0/packages/ember-metal/lib/map.js#L55

Edited by @Serabe with @locks consent: See #15041 (comment) for latest information as of 2017-07-13.
@pixelhandler
Copy link
Contributor

Not sure what actions are necessary regarding the comment We plan to clean it up and add many tests soon. means. What types of tests need to be added? I see https://github.com/emberjs/ember.js/blob/master/packages/ember-metal/tests/map_test.js#L512-L527 What does clean it up mean, e.g. remove, polish, reorganize?

@mixonic
Copy link
Member

mixonic commented Mar 26, 2017

@locks please provide more detail, or perhaps this can be closed.

Serabe added a commit to Serabe/ember.js that referenced this issue Apr 7, 2017
@Serabe
[CLEANUP] Remove ember-metal/map
193dc92 
`ember-metal/map` contained three private classes `OrderedSet`, `Map`,
`MapWithDefault`. They were not used anywhere in the codebase except for tests
or reexports.

Fixes emberjs#15041
@Serabe Serabe mentioned this issue Apr 7, 2017
Closed
@Serabe
Copy link
Member

Serabe commented Jul 13, 2017

I created this #15119 and last thing I know is that some consensus was reached in core, and someone would update on the topic, but cannot find anything in notes.

@mixonic
Copy link
Member

mixonic commented Jul 13, 2017

@Serabe from the conversation on that issue I think an addon is being requested as a transition plan for current users.

Would be great to land that PR. I'm still unclear about what this particular issue is tracking. Perhaps it has be superseded?

@locks locks self-assigned this Jul 20, 2017
@pixelhandler
Copy link
Contributor

@locks any update on this one?

@rwjblue
Copy link
Member

rwjblue commented Nov 5, 2017

Closing in favor of #15119

@rwjblue rwjblue closed this as completed Nov 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@locks locks

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mixonic @rwjblue @pixelhandler @locks @Serabe