-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
RFC 176 doc updates for ember-metal package #16154
RFC 176 doc updates for ember-metal package #16154
Conversation
@locks I am not sure about how to update the reference to ember-runtime package link.
https://github.com/emberjs/ember.js/blob/v2.18.0/packages/ember-metal/lib/core.js#L16 After 2.15 version of the docs, it seems ember-runtime removed (https://emberjs.com/api/ember/2.15/modules/ember-runtime). How to proceed? |
packages/ember-metal/lib/map.js
Outdated
@@ -186,7 +186,7 @@ class OrderedSet { | |||
|
|||
/** | |||
@method copy | |||
@return {Ember.OrderedSet} | |||
@return {OrderedSet} | |||
@private | |||
*/ | |||
copy() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on line 220, we still have @namespace
for Map. We should remove this line so Map shows up as Map
in the docs and not Ember.Map
packages/ember-metal/lib/map.js
Outdated
@@ -63,7 +63,7 @@ class OrderedSet { | |||
/** | |||
@method create | |||
@static | |||
@return {Ember.OrderedSet} | |||
@return {OrderedSet} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://github.com/ember-cli/ember-rfc176-data/blob/master/mappings.json its still Ember.OrderedSet
@@ -63,7 +63,7 @@ class OrderedSet { | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make the @module
. @ember/map
and not ember
packages/ember-metal/lib/run_loop.js
Outdated
@@ -161,7 +161,7 @@ run.join = function() { | |||
}); | |||
``` | |||
|
|||
In this example, we use Ember.run.bind to bind the setupEditor method to the | |||
In this example, we use `@ember/runloop/bind` to bind the setupEditor method to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just use bind
packages/ember-metal/lib/watching.js
Outdated
@@ -21,7 +21,7 @@ import { | |||
invokes `Ember.propertyWillChange` and `Ember.propertyDidChange`. This is the | |||
primitive used by observers and dependent keys; usually you will never call | |||
this method directly but instead use higher level methods like | |||
`Ember.addObserver()` | |||
`@ember/object/observers/addObserver` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just addObserver
should do.
@@ -3,7 +3,7 @@ import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; | |||
|
|||
let obj; | |||
|
|||
moduleFor('Ember.get with path', class extends AbstractTestCase { | |||
moduleFor('@ember/object/get with path', class extends AbstractTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get with path
@@ -1,7 +1,7 @@ | |||
import { getProperties } from '../..'; | |||
import { moduleFor, AbstractTestCase } from 'internal-test-helpers'; | |||
|
|||
moduleFor('Ember.getProperties', class extends AbstractTestCase { | |||
moduleFor('@ember/object/getProperties', class extends AbstractTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getProperties
@@ -1,8 +1,8 @@ | |||
import { isBlank } from '..'; | |||
|
|||
QUnit.module('Ember.isBlank'); | |||
QUnit.module('@ember/utils/isBlank'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isBlank
|
||
QUnit.test('Ember.isEmpty', function(assert) { | ||
QUnit.test('@ember/utils/isEmpty', function(assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isEmpty
Generally, I'd like for functions like |
@Karthiick we should delete that block of documentation for |
276182b
to
7526ba1
Compare
@toddjordan I have made the changes. Let me know, if i missed something. |
packages/ember-metal/lib/map.js
Outdated
@@ -2,7 +2,7 @@ import { assert } from 'ember-debug'; | |||
import { guidFor } from 'ember-utils'; | |||
|
|||
/** | |||
@module ember | |||
@module ember/map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to @ember/map
and we are good
@Karthiick this is great. I commented on just one more change and then I will merge. |
7526ba1
to
497196e
Compare
@toddjordan its done! |
Looks great! Will merge when the CI passes. Thanks @Karthiick !! |
#15723