genkgo / ember-localforage-adapter

Offline usage for Ember Data, based on localstorage adapter, but now uses Mozilla's localforage as data source
Other
133 stars 26 forks source link

Ember Data v1.0.0-beta.16.x deprecation #18

Closed Chun-Yang closed 9 years ago

Chun-Yang commented 9 years ago

I get the following deprecation warnings when switching to Ember Data v1.0.0-beta.16

Using DS.Snapshot.get() is deprecated. Use .attr(), .belongsTo() or .hasMany() instead.
Usage of `snapshot.constructor` is deprecated, use `snapshot.type` instead.

I would like to make a pull request. Or someone is working this already?

frederikbosch commented 9 years ago

@Chun-Yang The last release of this package was done with beta 14 of Ember Data. So if you could provide a pull request, that would be great. I will then merge as quickly as possible. Thanks anyway for your providing useful feedback!

Chun-Yang commented 9 years ago

@frederikbosch I can not run ember server using the previous ember-cli, so I updated ember-cli. Also ember is also updated to 1.10.0. Tests all passed.

Please tell me what you think about it.

frederikbosch commented 9 years ago

Looks great. I'll wait for Travis before merging and tagging.

Chun-Yang commented 9 years ago

It seems there are some jshint errors:

modules/ember-localforage-adapter/adapters/localforage.js: line 4, col 1, 'import' is only available in ES6 (use esnext option).
modules/ember-localforage-adapter/adapters/localforage.js: line 6, col 1, 'export' is only available in ES6 (use esnext option).
modules/ember-localforage-adapter/adapters/localforage.js: line 410, col 19, ['_embedded'] is better written in dot notation.

I can solve the ['_embedded'] one, can you configure travis to use esnext option?

frederikbosch commented 9 years ago

@Chun-Yang I think you can do that by changing .travis.yml: it is in the root of the project.

Chun-Yang commented 9 years ago

OK, all passed.

frederikbosch commented 9 years ago

Great! But now there are two jshint file: one in tests/.jshintrc and one in the root. I do not believe the latter should be necessary, or is it?

Chun-Yang commented 9 years ago

Just tested it. It is not necessary. Already deleted in the PR.

frederikbosch commented 9 years ago

Perfect, thanks for the help, much appreciated!

frederikbosch commented 9 years ago

Just pushed a new release, sorry for the delay.

Chun-Yang commented 9 years ago

Hi @frederikbosch , I did not use the 'ember-data' way to serialize record in the previous PR. I did another PR to correct it. #21

Please consider to merge it. Thanks.

frederikbosch commented 9 years ago

@Chun-Yang Done. Including new version release. Thanks for your help!