Skip to content

[RFC#236] Deprecate Ember.String.loc and {{loc}}#19211

Merged
rwjblue merged 1 commit intomasterfrom
deprecation-ember-string-loc
Nov 11, 2020
Merged

[RFC#236] Deprecate Ember.String.loc and {{loc}}#19211
rwjblue merged 1 commit intomasterfrom
deprecation-ember-string-loc

Conversation

@locks
Copy link
Copy Markdown
Contributor

@locks locks commented Oct 16, 2020 •
edited
Loading

Part of RFC#236 - Ember.String deprecation.

Tasks

  • deprecate Ember.String.loc
    • make sure deprecation URL is published and matches
  • deprecate {{loc}}

@locks locks mentioned this pull request Oct 16, 2020
Closed
17 tasks
@locks
Copy link
Copy Markdown
Contributor Author

locks commented Oct 17, 2020 •
edited
Loading

I think I'm missing something obvious in the tests but alas, I cannot figure it out at the moment. If anyone can lend a hand, please do 🙏

Died on test #4 generateTest@http://localhost:7357/7816/tests/ember-tests.js:93037:15
setupTestClass@http://localhost:7357/7816/tests/ember-tests.js:93023:16
moduleFor/<@http://localhost:7357/7816/tests/ember-tests.js:92976:21
setupQUnit/QUnit.module/<@http://localhost:7357/7816/tests/ember-tests.js:92429:17
processModule@http://localhost:7357/7816/tests/qunit/qunit.js:1200:16
module$1@http://localhost:7357/7816/tests/qunit/qunit.js:1225:17
setupQUnit/QUnit.module@http://localhost:7357/7816/tests/ember-tests.js:92421:14
moduleFor@http://localhost:7357/7816/tests/ember-tests.js:92975:11
@http://localhost:7357/7816/tests/ember-tests.js:36309:38
internalRequire@http://localhost:7357/7816/tests/ember.js:64:14
require@http://localhost:7357/7816/tests/ember.js:97:14
@http://localhost:7357/7816/tests/index.html?hidepassed&moduleId=e12f7341&moduleId=d6ae189f&testId=1e6014bc:181:61
: loc is deprecated.@ 33 ms

@locks locks changed the title Deprecate Ember.String.loc and {{loc}} [RFC#236] Deprecate Ember.String.loc and {{loc}} Oct 17, 2020
@locks
Copy link
Copy Markdown
Contributor Author

locks commented Oct 17, 2020

It seems I was not using the expectDeprecation wrapper correctly. The test suite should hopefully now pass.

rwjblue
rwjblue reviewed Oct 17, 2020
View reviewed changes
packages/@ember/string/index.ts Outdated
@public
*/
export function loc(str: string, formats: any[]): string {
deprecate('loc is deprecated.', false, {
Copy link
Copy Markdown
Member

@rwjblue rwjblue Oct 17, 2020

Choose a reason for hiding this comment

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

This is pretty short and sweet. Do you think we need to say something a bit longer here?

Copy link
Copy Markdown
Contributor Author

@locks locks Oct 17, 2020

Choose a reason for hiding this comment

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

I'm finishing the deprecation guide and then I'll update the ember.js side with the final link and text!

mehulkar
mehulkar reviewed Oct 18, 2020
View reviewed changes
packages/@ember/string/index.ts Outdated
url: 'https://deprecations.emberjs.com/v3.x#toc_ember-string-loc',
since: {
available: '3.25',
},
Copy link
Copy Markdown
Contributor

@mehulkar mehulkar Oct 18, 2020

Choose a reason for hiding this comment

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

🎉 ❤️

@locks locks requested review from MelSumner and jenweber October 23, 2020 15:18
@locks
Copy link
Copy Markdown
Contributor Author

locks commented Oct 23, 2020

@rwjblue description updated. Also pinged other learning team members.

jaredgalanis
jaredgalanis approved these changes Oct 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jaredgalanis jaredgalanis left a comment

Choose a reason for hiding this comment

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

Looks good 👍, one small thing.

@locks locks marked this pull request as ready for review October 25, 2020 20:37
locks
locks commented Oct 27, 2020
View reviewed changes
rwjblue
rwjblue requested changes Oct 27, 2020
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.

Looks good, just needs that one change of for in the deprecation.

@locks locks force-pushed the deprecation-ember-string-loc branch from 6f30002 to ee7385d Compare October 27, 2020 12:46
@locks locks requested a review from rwjblue November 10, 2020 02:40
@rwjblue
Copy link
Copy Markdown
Member

rwjblue commented Nov 11, 2020

@locks - Can you squash down the commits?

@locks locks force-pushed the deprecation-ember-string-loc branch from 26cdd3f to 452c392 Compare November 11, 2020 18:36
@locks locks force-pushed the deprecation-ember-string-loc branch from 452c392 to c7526f1 Compare November 11, 2020 18:36
@locks
Copy link
Copy Markdown
Contributor Author

locks commented Nov 11, 2020

Rebased and squashed!

@rwjblue rwjblue merged commit 7b600a9 into master Nov 11, 2020
@rwjblue rwjblue deleted the deprecation-ember-string-loc branch November 11, 2020 19:26
@locks locks mentioned this pull request Jan 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rwjblue rwjblue rwjblue approved these changes

@jenweber jenweber Awaiting requested review from jenweber

@MelSumner MelSumner Awaiting requested review from MelSumner

+2 more reviewers

@mehulkar mehulkar mehulkar left review comments

@jaredgalanis jaredgalanis jaredgalanis 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.

4 participants

@locks @rwjblue @mehulkar @jaredgalanis