search

locks / ember-localstorage-adapter

Name says it all.
MIT License
466 stars 156 forks source link

Use `includes` instead of `contains` if possible #213

Closed jacobq closed 7 years ago

jacobq commented 7 years ago

I believe this prevents an ember-data 2.x deprecation warning while staying compatible with the ember-data 1.x API.

jacobq commented 7 years ago

Does that failing Travis CI check just mean I forgot that all ifs need {}s in this project or is there something else that actually breaks? I'll try adding the curlies....

locks commented 7 years ago

Can you instead include https://github.com/rwjblue/ember-runtime-enumerable-includes-polyfill as recommended in the deprecation guide? Appreciated!

jacobq commented 7 years ago

Sure -- I'll check it out. I wasn't familiar with that....just getting annoyed / scared by deprecation warnings in my app and being a bit pedantic about taking care of them :)

jacobq commented 7 years ago

Better?

locks commented 7 years ago

Can you squash the commits? Then we're off to the races!

jacobq commented 7 years ago

I'm a bit embarrassed that I don't know the "right"/easy way to do squash commits in a PR, so I rebased my patch-1 branch and force pushed it up. (Figuring nobody else would be using that branch for anything else, which seems like a valid case for forcing.) If you have recommendations, suggestions, or links for me to go read that explain better practice feel free to share, and I will do my best to bear that in mind in the future.

locks commented 7 years ago

I believe the usual go to article is https://github.com/ginatrapani/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit. Let's carry this convo off-thread! :)