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

[RFC 297] Deprecation Ember.Logger #16231

Closed
7 of 8 tasks
locks opened this issue Feb 9, 2018 · 9 comments
Closed
7 of 8 tasks

[RFC 297] Deprecation Ember.Logger #16231

locks opened this issue Feb 9, 2018 · 9 comments

Comments

@locks
Copy link
Contributor

locks commented Feb 9, 2018

RFC: emberjs/rfcs#297

While the RFC is not merged yet, it was recently moved into Final Comment Period and there is general support for this to land, so we feel confident that work can commence in preparation.

Tasks

  • Deprecate APIs in Ember.js - PR
  • Develop and publish codemod
  • Write Deprecation Guide
    • emberjs/website - PR
    • ember-learn/deprecation-app - PR
  • Review API docs
  • Review Guides - PR
  • Review top addons
    • Ember Data - PR
    • ember-resolver - PR
@lupestro
Copy link

lupestro commented Feb 16, 2018

Locally, as of last weekend...

  • I currently have the two passing references to Ember.Logger out of the Guides. These could probably go into a pull request with or without the RFC.
  • I have a draft entry for the Deprecation Guide written, but the final details will depend on release timing.
  • I've also coded the change for the Ember.Debug to not use Ember.Logger and I've coded the deprecation itself, although I think this may need to go back and wrap all of this in a feature flag.
  • I've reworked tests that intercepted Ember.Logger to intercept console instead, but there are maybe four router tests that never set it back but exit after calling bootApplication(); and do a run() of a transition or two, which concerns me a little.
  • All existing tests are passing, but, due to the issue with the router tests, I need to verify whether the test leakage to the browser console is identical before and after my change.

...I get busy elsewhere during the week. I'm hoping to have the deprecation itself completed, feature flag and all, this weekend, or before the RFC exits FCP.

I'm not sure of the most comprehensive way to review the API docs besides bulk searches on the source, which I've been doing anyway. I think I've done the right things in the code comments to properly mark the deprecation but have no idea how to build the API docs to see the final effect.

That leaves the codemod and the review of the addons, for which I will need a little help.

@lupestro
Copy link

lupestro commented Feb 18, 2018

So far, we have the following PRs:
https://github.com/emberjs/guides/pull/2230 - A couple of Logger references - merged and closed already
https://github.com/emberjs/ember.js/pull/16250 - Deprecation code
https://github.com/emberjs/data/pull/5356- One reference to Logger - merged and closed already
https://github.com/emberjs/website/pull/3197 - Corresponding deprecation page
https://github.com/ember-learn/deprecation-app/pull/91 - Corresponding deprecation page

@lupestro
Copy link

I encountered a couple of unexpected speed-bumps using console in Edge and IE11. The simple page in the attachment can be used to easily observe the console uses that pose a problem and their easy workarounds. Try it in Edge and IE11 with and without the developer tools up, then try it in other browsers. tryconsole.zip

@lupestro
Copy link

Uh-oh. I just realized, walking emberobserver, that we didn’t have ember-resolver on the above list. So many projects...

@locks
Copy link
Contributor Author

locks commented Feb 23, 2018

@lupestro not sure I understand?

@lupestro
Copy link

In wake of our discussion @locks, here’s the issue over on ember-resolver:
https://github.com/ember-cli/ember-resolver/issues/223

@rwjblue
Copy link
Member

rwjblue commented Mar 24, 2018

Great job!!

@scottkidder
Copy link

Is there a codemod or any other discussion for this deprecation?

@lupestro
Copy link

lupestro commented Jun 4, 2018

There has been discussion for it. We captured prior discussion in the RFC itself, the PR for the RFC carries further discussion, and the deprecation text describes what needs to change, and what you have to do to deal with the one or two remaining oddities of console in IE11 and Edge.

There is no codemod to automate changing the code, as the work can involve a little judgment, but the changes are generally very local and simple, as the deprecation text tries to show. The RFC references some of the code that we examined in determining the impact of the change. In all the code we looked at, it seemed straightforward to find and make any needed changes by hand.

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

No branches or pull requests

4 participants