Skip to content
This repository was archived by the owner on Mar 22, 2019. It is now read-only.

deprecate Ember.K#2774

Merged
rwjblue merged 1 commit intoemberjs:masterfrom
givanse:deprecate-ember-k
Dec 26, 2016
Merged

deprecate Ember.K#2774
rwjblue merged 1 commit intoemberjs:masterfrom
givanse:deprecate-ember-k

Conversation

@givanse
Copy link
Copy Markdown
Contributor

@givanse givanse commented Dec 23, 2016 •
edited
Loading

deprecate Ember.K according to emberjs/ember.js#14746

rendered view

This was referenced Dec 23, 2016
Closed
Merged
cibernox
cibernox approved these changes Dec 23, 2016
View reviewed changes
Copy link
Copy Markdown
Contributor

@cibernox cibernox left a comment

Choose a reason for hiding this comment

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

It seems a very clear explanation to me. I'll ping someone with merge rights /cc @locks

@locks
Copy link
Copy Markdown
Contributor

locks commented Dec 23, 2016

We'd like the explanation to lean much more in the direction of --empty. We think that most people aren't using the chaining property of Ember.K and it's preferable to go with the simpler option.
@givanse could you tweak it a little bit so that it's more "oh, here's the command with the --empty option. P.S. if you're using chaining, there's --return-this"?

@givanse
Copy link
Copy Markdown
Contributor Author

givanse commented Dec 23, 2016 •
edited
Loading

PR updated with added emphasis on the --empty flag
https://github.com/givanse/website/blob/d2b6d8d435dd7d156dc7b0e0c83b3af45f984651/source/deprecations/v2.x.html.md#emberk thoughts?

@locks locks requested a review from mixonic December 23, 2016 16:30
rwjblue
rwjblue suggested changes Dec 23, 2016
View reviewed changes
Copy link
Copy Markdown
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

The changes added here look good (thank you!), but can you add a header above your changes for:

### Deprecations Added in 2.12

(since this deprecation will land in 2.12 which is the current emberjs/ember.js#master branch)

@givanse
Copy link
Copy Markdown
Contributor Author

givanse commented Dec 23, 2016 •
edited
Loading

Header added
https://github.com/givanse/website/blob/0053f6e7f5d4556937e0b20dd5ca491c306f90cb/source/deprecations/v2.x.html.md#deprecations-added-in-212

I don't know what to do about the failed deploy/netlify

You don't have access to this build log.

@locks
Copy link
Copy Markdown
Contributor

locks commented Dec 23, 2016

@givanse it's not your fault, we're trying it out and all PRs are failing for some reason!

@rwjblue rwjblue merged commit 6372840 into emberjs:master Dec 26, 2016
@givanse givanse deleted the deprecate-ember-k branch December 26, 2016 02:25
@givanse givanse mentioned this pull request Sep 28, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mixonic mixonic Awaiting requested review from mixonic

3 more reviewers

@stefanpenner stefanpenner stefanpenner approved these changes

@rwjblue rwjblue rwjblue approved these changes

@cibernox cibernox cibernox approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@givanse @locks @stefanpenner @rwjblue @cibernox