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

3.0: Support Ember/Ember-Data 1.13 and up only #95

Merged
merged 7 commits into from
Dec 13, 2015

Conversation

rsutphin
Copy link
Collaborator

@rsutphin rsutphin commented Dec 5, 2015

Builds on @fivetanley's PR #87 to drop support for Ember & Ember-Data before version 1.13. Includes changes to the ember-try & travis config to test on 1.13 and all 2.x versions so far.

Since this is a backwards-incompatible change, it bumps the package version to 3.0.

Given our limited test suite, I've normally been testing changes in my app that uses the adapter. However, that app is still on Ember 1.12, so I can't test these changes. I'd appreciate it if anyone who's interested could give it a try in their apps and let me know if there are problems.

@broerse
Copy link
Collaborator

broerse commented Dec 5, 2015

This looks good. Perhaps add https://github.com/rwjblue/ember-cli-version-checker for ember-data?

@nolanlawson
Copy link
Member

I'm all for dropping support for old versions, if it makes development easier. That said, we should probably have better tests. Also, is there a way to communicate via code which version of Ember Data we support? Something like npm peerDependencies?

@mattmarcum
Copy link
Contributor

👍 code looks good

@broerse
Copy link
Collaborator

broerse commented Dec 8, 2015

broerse/ember-cli-blog@438d2af

Works!

@fivetanley
Copy link
Contributor

@nolanlawson beginning in Ember Data 2.4, Ember Data will be a full-fledged Ember add-on like Ember Pouch. You can use peerDependencies in that fashion. Until then, I think you would have to communicate this at runtime by asserting against DS.VERSION.

@broerse
Copy link
Collaborator

broerse commented Dec 8, 2015

This is not exactly right — it would accept 2.4 as a bower dep — but it's as
close as I can get using ember-cli-version-checker's documented API.
@rsutphin rsutphin force-pushed the support-1.13-and-up-only branch from 48d7f69 to 38b21a5 Compare December 8, 2015 16:53
@rsutphin
Copy link
Collaborator Author

rsutphin commented Dec 8, 2015

Okay, I added a version check. I used ember-cli-version-checker. It would have been cleaner to use a check against DS.VERSION since we wouldn't have to have knowledge of how ember-data is distributed at different versions. But I don't see a way to do that because I can't use semver (or other npm libraries) at runtime without a lot of complexity that seems unnecessary for a simple version check.

@broerse
Copy link
Collaborator

broerse commented Dec 9, 2015

@rsutphin The ember-data version check look good. Should we also add a version check for ember?

@rsutphin
Copy link
Collaborator Author

rsutphin commented Dec 9, 2015

I don't think we need a version check for ember. Ember-pouch mainly interacts with ember-data. It doesn't use any complex or subtle ember features — whatever version of ember works with the version of ember-data you have should be fine.

@broerse, thanks for checking in your app. Are there any further concerns before I merge this?

@broerse
Copy link
Collaborator

broerse commented Dec 9, 2015

@rsutphin It works OK for me but my app is still in 1.13.x. I see there are some issues with other modules and ember & ember-data 2.2.0. We could first double check 2.2.0.

I vote for merging now.

@mattmarcum
Copy link
Contributor

I just tried out this branch locally on ember js 2.2 and ember data 2.2.1 and everything worked fine.

@broerse
Copy link
Collaborator

broerse commented Dec 10, 2015

@mattmarcum Thanks for testing this!

@rsutphin
Copy link
Collaborator Author

@mattmarcum, thanks for testing.

I tested with HospitalRun. None of their tests that were passing before fail with this change. (I didn't run the full app, though.) I'm going to merge now.

Thanks for everyone's feedback.

rsutphin added a commit that referenced this pull request Dec 13, 2015
3.0: Support Ember/Ember-Data 1.13 and up only
@rsutphin rsutphin merged commit 4e28c84 into pouchdb-community:master Dec 13, 2015
@rsutphin
Copy link
Collaborator Author

Published as 3.0.0.

@rsutphin rsutphin deleted the support-1.13-and-up-only branch December 13, 2015 04:38
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.

5 participants