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

[DOC release] Add example and clear up wording for CP get/set #12619

Merged
merged 1 commit into from
Nov 16, 2015

Conversation

SaladFork
Copy link
Contributor

This PR makes the following changes to Ember.computed API documentation:

  • Changes the fullName getter function to match the example in the latest guides. This involved changing to a template string from a string concatenation.

  • Switches class property default values to be defined in init() instead (suggested)

  • Minor changes to wording and sentence structure, mostly adding articles.

  • Adds an example that uses the hash format with get and set

  • Removes confusing/incorrect phrase:

    The get function should accept two parameters, key and value.

My one concern is that the two examples are very similar except for a few lines. Would it be more helpful if the second example only included the computed property (rather than the whole class and usage example)?


```js
let Person = Ember.Object.extend({
firstName: 'Betty',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets put these in init.

init() {
  this._super(...arguments)
  this.firstName = 'Betty';
  this.lastName =  'Jones';
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, switched both examples to this style.

@SaladFork SaladFork force-pushed the doc-add-cp-getter-setter-example branch from 3df069e to 3b0baff Compare November 16, 2015 21:13
@stefanpenner
Copy link
Member

LGTM

@rwjblue
Copy link
Member

rwjblue commented Nov 16, 2015

@homu r+

@homu
Copy link
Contributor

homu commented Nov 16, 2015

📌 Commit 3b0baff has been approved by rwjblue

@homu
Copy link
Contributor

homu commented Nov 16, 2015

⚡ Test exempted - status

@homu homu merged commit 3b0baff into emberjs:master Nov 16, 2015
homu added a commit that referenced this pull request Nov 16, 2015
…rwjblue

[DOC release] Add example and clear up wording for CP get/set

This PR makes the following changes to `Ember.computed` API documentation:
 - Changes the `fullName` getter function to match [the example in the latest guides](http://guides.emberjs.com/v2.1.0/object-model/computed-properties/). This involved changing to a template string from a string concatenation.
 - Switches class property default values to be defined in `init()` instead ([suggested](#12619 (comment)))
 - Minor changes to wording and sentence structure, mostly adding articles.
 - Adds an example that uses the hash format with `get` and `set`
 - Removes confusing/incorrect phrase:

 > The `get` function should accept two parameters, `key` and `value`.

My one concern is that the two examples are very similar except for a few lines. Would it be more helpful if the second example only included the computed property (rather than the whole class and usage example)?
@SaladFork SaladFork deleted the doc-add-cp-getter-setter-example branch November 17, 2015 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants