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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Nov 11, 2020
Merged

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

merged 1 commit into from
Nov 11, 2020

Conversation

locks
Copy link
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
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
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
@@ -111,6 +112,16 @@ function _fmt(str: string, formats: any[]) {
@public
*/
export function loc(str: string, formats: any[]): string {
deprecate('loc is deprecated.', false, {
Copy link
Member

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
Contributor Author

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
Contributor

Choose a reason for hiding this comment

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

🎉 ❤️

@locks
Copy link
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
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.

packages/@ember/string/index.ts Outdated Show resolved Hide resolved
@locks locks marked this pull request as ready for review October 25, 2020 20:37
rwjblue
rwjblue requested changes Oct 27, 2020
View reviewed changes
Copy link
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
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
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

@mehulkar mehulkar mehulkar left review comments

@jaredgalanis jaredgalanis jaredgalanis approved these changes

@rwjblue rwjblue rwjblue approved these changes

@jenweber jenweber Awaiting requested review from jenweber

@MelSumner MelSumner Awaiting requested review from MelSumner

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