Adjust code samples to ember-cli #3174

Turbo87 · 2015-06-03T08:06:34Z

I'm adjusting the samples to match the blueprints of the ember-cli tool as discussed in #3139.
Closes #3155

wecc · 2015-06-03T08:40:07Z

packages/ember-data/lib/transforms/base.js

@@ -8,8 +8,10 @@
  Example

  ```javascript
+  import DS from 'ember-data';


Add // app/transforms/temperature.js here?

yep, good idea. done.

sly7-7 · 2015-06-03T09:04:49Z

packages/ember-data/lib/adapters/rest-adapter.js

@@ -99,7 +102,7 @@ import BuildURLMixin from "ember-data/adapters/build-url-mixin";
  property on the adapter:

  ```js
-  App.ApplicationAdapter = DS.RESTAdapter.extend({
+  export default DS.RESTAdapter.extend({


Maybe it's worth it to add the file name here too ?

bmac · 2015-06-03T10:20:38Z

packages/ember-data/lib/adapters/build-url-mixin.js

@@ -251,7 +251,7 @@ export default Ember.Mixin.create({
    endpoint of "/line_items/".

    ```js
-    App.ApplicationAdapter = DS.RESTAdapter.extend({
+    export default DS.RESTAdapter.extend({


// app/adapters/application.js

Turbo87 · 2015-06-03T11:47:50Z

@bmac @sly7-7 done

sly7-7 · 2015-06-03T12:04:19Z

@Turbo87 @bmac Maybe the header //app/adapters/application.jscould added in every snippet where we extend the adapter ? Or maybe it's enough to have it in one example ?

bmac · 2015-06-03T12:08:17Z

I think it should be in every example. Most people just look at the one api they are interested in instead of reading the whole page.

Turbo87 · 2015-06-03T12:09:21Z

@sly7-7 I felt like it might be too much visual clutter if we include it everywhere. It might also make people believe that this is only valid for the application adapter, which is obviously not true. I haven't found a good rule yet on when to include it, but I think if the example explains more then just a simple method or property (e.g. an interaction with a model) it should definitely be included.

sly7-7 · 2015-06-03T12:17:16Z

@Turbo87 It seems like you added it to all the snippets for serializers/embedded mixin, so for consistency, either we only have one per file, or one per snippets.
That beeing said, when I played with ember-cli and ember-data I remember me trying to find in which file the snippets should be included, so my opinion would be the same as @bmac.
Maybe we could add/replace an example with a per-type adapter, so that it would help to figure out it's not only valid for application adapter ?

bmac · 2015-06-03T19:23:15Z

We should look into the helper that the guides use to make their pretty file names and see if it needs to be added to the ember/website repo.

igorT · 2015-06-05T00:10:04Z

paging @rwjblue

bmac · 2015-06-05T01:10:43Z

Looks like the code is already on the website repo: emberjs/website@3bd5ef6

bmac · 2015-06-05T01:42:06Z

Looks like I spoke too soon the helper works but looks ugly. I've opened a pr to fix this: emberjs/website#2192.

@Turbo87 Do you mind updating this pr to remove the all of the filename comments and replace the js after the 3 backticks with the filename.

```js
// app/adapters/application.js
code...

->

```app/adapters/application.js
code...

Turbo87 · 2015-06-05T07:31:38Z

@bmac @sly7-7 done

sly7-7 · 2015-06-05T08:10:44Z

packages/ember-data/lib/system/model/attributes.js

      firstName: attr('string'),
      lastName: attr('string'),
      birthday: attr('date')
    });

-    var attributes = Ember.get(App.Person, 'attributes')
+    var attributes = Ember.get(Person, 'attributes')


not sure what to do in those cases, where we consume the class.
Either use the app/models/person.js header, either replace the class declaration by an import statement like import Person from app/models/person ?

Turbo87 · 2015-06-05T08:10:45Z

packages/ember-data/lib/system/model/attributes.js


-    App.Person = DS.Model.extend({
+    var Person = DS.Model.extend({


@sly7-7 the problem is the Ember.get() call below. I'm not sure how this is supposed to work with ember-cli now. Is there something similar that could replace it?

I think you could still do
import Ember from 'ember'
then call the Ember.get()
@bmac c/d ?

sly7-7 · 2015-06-05T08:30:30Z

Well, this probably needs someone else eyes, but LGTM
Thanks a lot @Turbo87 awesome work !! ❤️

ember-cli is the recommended way of doing things now and should have priority

bmac · 2015-06-05T09:23:21Z

README.md

-var attr = DS.attr;
-var hasMany = DS.hasMany;
-var belongsTo = DS.belongsTo;
+// app/models/blog-post.js


Lets move this to the header.

Ah nevermind this is the README.md.

do you mean ```app/models/blog-post.js? that won't work here since it will be rendered by GitHub, not the website.

bmac · 2015-06-05T09:30:02Z

Looks good @Turbo87 🎉

Adjust code samples to ember-cli

Turbo87 · 2015-06-05T09:30:48Z

cool, thanks for merging!

Turbo87 force-pushed the ember-cli branch from 005bf88 to dc1c076 Compare June 3, 2015 08:15

wecc reviewed Jun 3, 2015
View reviewed changes

Turbo87 force-pushed the ember-cli branch 2 times, most recently from 0f5f5a3 to 071990c Compare June 3, 2015 08:59

Turbo87 changed the title ~~[WIP] Adjust code samples to ember-cli~~ Adjust code samples to ember-cli Jun 3, 2015

sly7-7 reviewed Jun 3, 2015
View reviewed changes

sly7-7 mentioned this pull request Jun 3, 2015

Update docs to ember-cli style #3155

Closed

bmac reviewed Jun 3, 2015
View reviewed changes

Turbo87 force-pushed the ember-cli branch from 071990c to 4038241 Compare June 3, 2015 11:47

Turbo87 force-pushed the ember-cli branch 2 times, most recently from d155bde to 82f6067 Compare June 5, 2015 07:30

Turbo87 added 5 commits June 5, 2015 09:34

transforms: Adjust code samples to ember-cli

87e4767

serializers: Adjust code samples to ember-cli

2b796c7

adapters: Adjust code samples to ember-cli

b283340

system/adapter: Adjust code samples to ember-cli

09b20c7

system/many-array: Adjust code samples to ember-cli

36ee20c

Turbo87 force-pushed the ember-cli branch from 82f6067 to 36ee20c Compare June 5, 2015 07:34

system/store: Adjust code samples to ember-cli

92401c8

Turbo87 force-pushed the ember-cli branch from a1840cf to 325c402 Compare June 5, 2015 07:58

sly7-7 reviewed Jun 5, 2015
View reviewed changes

Turbo87 reviewed Jun 5, 2015
View reviewed changes

Turbo87 added 2 commits June 5, 2015 10:21

system/model: Adjust code samples to ember-cli

97f39ac

system/relationships: Adjust code samples to ember-cli

0aabfd7

Turbo87 force-pushed the ember-cli branch from 9345688 to 0aabfd7 Compare June 5, 2015 08:21

Turbo87 added 2 commits June 5, 2015 10:31

README: Adjust code samples to ember-cli

d963e37

README: Move ember-cli example above globals

08bb942

ember-cli is the recommended way of doing things now and should have priority

bmac reviewed Jun 5, 2015
View reviewed changes

bmac added a commit that referenced this pull request Jun 5, 2015

Merge pull request #3174 from Turbo87/ember-cli

6a03323

Adjust code samples to ember-cli

bmac merged commit 6a03323 into emberjs:master Jun 5, 2015

Turbo87 deleted the ember-cli branch June 5, 2015 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust code samples to ember-cli #3174

Adjust code samples to ember-cli #3174

Turbo87 commented Jun 3, 2015

wecc Jun 3, 2015

Turbo87 Jun 3, 2015

sly7-7 Jun 3, 2015

bmac Jun 3, 2015

Turbo87 commented Jun 3, 2015

sly7-7 commented Jun 3, 2015

bmac commented Jun 3, 2015

Turbo87 commented Jun 3, 2015

sly7-7 commented Jun 3, 2015

bmac commented Jun 3, 2015

igorT commented Jun 5, 2015

bmac commented Jun 5, 2015

bmac commented Jun 5, 2015

Turbo87 commented Jun 5, 2015

sly7-7 Jun 5, 2015

Turbo87 Jun 5, 2015

Turbo87 Jun 5, 2015

sly7-7 Jun 5, 2015

sly7-7 commented Jun 5, 2015

bmac Jun 5, 2015

bmac Jun 5, 2015

Turbo87 Jun 5, 2015

bmac commented Jun 5, 2015

Turbo87 commented Jun 5, 2015


		App.Person = DS.Model.extend({
		var Person = DS.Model.extend({

Adjust code samples to ember-cli #3174

Adjust code samples to ember-cli #3174

Conversation

Turbo87 commented Jun 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Turbo87 commented Jun 3, 2015

sly7-7 commented Jun 3, 2015

bmac commented Jun 3, 2015

Turbo87 commented Jun 3, 2015

sly7-7 commented Jun 3, 2015

bmac commented Jun 3, 2015

igorT commented Jun 5, 2015

bmac commented Jun 5, 2015

bmac commented Jun 5, 2015

Turbo87 commented Jun 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sly7-7 commented Jun 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmac commented Jun 5, 2015

Turbo87 commented Jun 5, 2015