-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Remove run and computed dot access #19653
Remove run and computed dot access #19653
Conversation
cafda2b
to
01aa1f3
Compare
bc8e880
to
975b18d
Compare
@@ -25,10 +25,6 @@ export default function confirmExport(Ember, assert, path, moduleId, exportName, | |||
desc = null; | |||
} | |||
|
|||
if (isDeprecated) { | |||
expectDeprecation(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expectDeprection()
called without a callback sets up an expectation that a deprecation occurs anywhere during the test run. As confirmExport
is called many times in the re-export test, this means if any one re-export* was expecting a deprecation satisfaction of that expectation would mean any later expectation was satisfied regardless of a new occurrence.
TLDR, https://github.com/emberjs/ember.js/pull/19653/files#diff-a9c227f21c32919d338aab29999b3ceaf7ca894a7bd3d7183ea81eda7578bdafL307 was a false positive. There was never a deprecation being issued for expandProperties
, but the test passed because the computed macro tests did have deprecations.
Since confirmExport
is not being used in a manner compatible with isDeprecated
being an argument, and since there are no uses of it with that argument being true
after this PR lands, I have removed the functionality from confirmExport
. We can re-introduce it correctly when needed.
975b18d
to
e8d7885
Compare
@@ -25,9 +25,7 @@ moduleFor( | |||
assert, | |||
path, | |||
moduleId, | |||
exportName, | |||
isDeprecated, | |||
`Ember.${path} exports correctly` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 7th argument (a string description) is not used. Here it is removed.
Note: also see https://github.com/emberjs/ember.js/pull/19653/files#r671884855 regarding the removal of isDeprecated
on the line above.
e8d7885
to
5b044de
Compare
5b044de
to
b005bc1
Compare
Part of #19617
Removing this code there is a bit of stuff adjacent to it regarding Ember global access. I haven't touched the Ember global stuff here, but this should probably land first and we can rip more code out during the Ember global removal.